Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor event triggering #1697

Merged
merged 8 commits into from Apr 20, 2019

Conversation

@fmatthew5876
Copy link
Contributor

commented Mar 13, 2019

Depends on #1694

Finally we bring back the touch, collision, and autostart trigger fix changes.

Tests #1626

  • 1-29 tested
  • 2.20 - EV01 stop_count is 2 instead of 3. - Due to unsolved issue in #1624 Test 9.2

Fix: #886
Fix: #1626
Fix: #1696
Fix: #1732

@fdelapena fdelapena added this to the 0.6.1 milestone Mar 13, 2019

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch 9 times, most recently from 63fa6a1 to cef87eb Mar 13, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

This is now ready for review. All the test cases pass.

@fdelapena

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

the Better diff isn't really better, wrong base branch I guess.

The commits starting from "Refactor touch collision" are new.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch 4 times, most recently from 1c1d271 to 41b8d71 Mar 27, 2019

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch from 41b8d71 to 031fcd3 Apr 3, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

"Ractor action and layer same triggering " <- Guess first is "Refactor"?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch from 031fcd3 to 36e621e Apr 7, 2019

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch from 36e621e to 39cec1b Apr 9, 2019

src/game_event.cpp Show resolved Hide resolved

fmatthew5876 added some commits Feb 9, 2019

Refactor touch collision
* Touch triggers when movement completes
* Triggers at end of move route only if last command was a move
* Remove Player::CancelMoveRoute and devirtualize
Refactor action and layer same triggering
* Refactor action triggering
* Fix counter behavior
    * Up to 3 counters
    * Fix behavior with touch and action events and counters
* Add OnMoveFailed() for triggering layer same collisions
* Remove IsValid() check for same layer event checks
Collision check adjustments
The test cases seem to indicate collision checks are
done before and after movement, but only under certain conditions
Fix for events with stop_count and collision trigger
When an event has layer=same and trigger=collision and fails
to step onto the player's position, stop_count gets reset to 0.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_trigger branch from 39cec1b to 5b4233e Apr 15, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Apr 15, 2019

Who wants to create a flow chart about event triggering? ;)

Well this is another one that can't be really reviewed through code but through observation, will move through all interesting, old issues and see if any regressed.

@Ghabry

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Still working:

#331
#669
#687
#1051

@Ghabry

Ghabry approved these changes Apr 20, 2019

Copy link
Member

left a comment

Well I can't really decide if this is all correct or not, the evidence says: This is not worse than before.

@carstene1ns carstene1ns merged commit 88c5a0d into EasyRPG:master Apr 20, 2019

7 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Wii (SDL1) Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.