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

Move Route refactors #1601

Merged
merged 32 commits into from Mar 10, 2019

Conversation

@fmatthew5876
Copy link
Contributor

commented Jan 5, 2019

This PR is refactor SaveMapEventBase move route related chunks to behave more like RPG_RT and improve support loading save games correctly on any frame of movement.

This still needs a lot more testing and probably more changes to be correct.

Detailed test cases and research can be found in #1567

Fix #1267
Fix #1293
Fix #1346
Fix #1429
Fix #1585
Fix #1587
Fix #1593
Fix #1604
Fix #1654
Fix #1671

Breaks #579
Breaks #937

Behaviors of stop_count

  • Continues ticking through the end of the last move route command into infinity, does not reset
  • Resets every time the player takes a step from the keyboard
  • Initialized to 0 on the first frame of a new game
  • Does not reset when we change maps
  • Move route advances after stop_count == max_stop_count
  • Initialized to 65535 when a SetMoveRoute command is processed.
  • Initialized to 0 when the next move command is processed, and then advances to 1 on the same frame.
  • Move routes are not processed on the first frame of a new map, and so stop_count will not tick for the first frame. This means it'll be 0 if on the first frame you do have a parallel event do OpenSaveMenu or 65535 if you do SetMoveRoute, OpenSaveMenu. The behavior affects the Player and Vehicles but not map events!
  • stop_count ticks while any (player, event, or vehicle) is idle and has no move route, unless an event is active in the foreground (autostart, talk).
  • Does not tick for vehicles not on the current map. Stays frozen to whatever the previous value was when you teleport away from a map that had the vehicle.
  • Does not tick for events which don't have any active page.
  • If the last move command is a step, we ignore max_stop_count and cancel the move route as soon as the move finishes, reseting back to idle state.
  • If the last move command is a jump, we ignore max_stop_count and cancel the move route as soon as the move finishes, reseting back to idle state.
  • When player vehicle > 0 and aboard = 1, that vehicle stop_count is frozen.
  • Does not change when you talk to an event and that event changes direction (or doesn't change)
  • SetEventLocation and SetVehicleLocation do not reset stop_count
  • Teleport does not reset stop_count

Behaviors of max_stop_count

  • Player and vehicle initialized to 0 on new game.
  • Map event initialized to stepping value of event's move_frequency.
  • Step: move_frequency == 8 ? 0 : (2 ^ (9 - move_frequency))
  • Turn: move_frequency == 8 ? 0 : (2 ^ (8 - move_frequency))
  • Wait: 20 + (move_frequency == 8 ? 0 : (2 ^ (8 - move_frequency)))
  • When a SetMoveRoute command is processed, initialized to stepping value for the move_frequency of the move route.
  • After move route finishes, resets to stepping value of the original_move_frequency
  • Does not change when the player steps from player controls (remains 0 when walking around after new game, or 128 when walking around after the Player has had 1 move route since the game started).

Behaviors of move_frequency

  • Player defaults to 2 (empty chunk) and resets to 2 after move route finishes
  • Vehicle defaults to 2 (empty chunk) and resets to 2 after move route finishes
  • Event defaults to active page move_frequency and resets to it after move route finishes
  • Event defaults to 2 (empty chunk) if no active page
  • When loading a save during an active move route, Player move_frequency resets to 0 after the move route finishes. (RPG_RT bug)
    • Fixed in Player
  • When loading a save during an active move route, Vehicle move_frequency resets to 0 after the move route finishes. (RPG_RT bug)
    • Fixed in Player
  • Increase/decrease frequency commands don't last after move route finishes

Behaviors of move_commands

  • Populated with array of commands when move route starts
  • Not cleared after move route finishes. Remains forever until next move route.

Behaviors of move_route_index

  • Starts at 0
  • Resets to 0 when new move route starts
  • When move command is being processed, points to the next move command, or move_commands.size() if this is the last move command.
  • If the last move command is a step, resets to 0 when move route finishes
  • If the last move command is not a step, remains unchanged when move route finishes

Behaviors of move_route_overwrite

  • Set 1 when move route starts
  • Set to 0 on the frame after move route finishes

Behaviors of jumping

  • Set to 1 on the frame the jump starts
  • Set to 0 on the last frame of the jump (when remaining_step = 0). This is different than most things which trigger on the frame after the movement.
  • SetEventLocation and SetVehicleLocation do not reset jumping (RPG_RT bug ❗️ )
    • Fixed in Player
  • Teleport does not reset jumping (RPG_RT bug ❗️ )
    • Fixed in Player

Behaviors of remaining_step

  • SetEventLocation and SetVehicleLocation reset remaining_step
  • Teleport resets remaining_step

Behaviors of anim_count

  • When animation_type is fixed_graphic or step_frame_fix, anim_count is always 0.
  • limit non-continuous = [11, 9, 7, 5, 4, 3] (based on move_speed)
  • limit continuous = [15, 11, 9, 7, 6, 5] (based on move_speed)
  • limit spin = [23, 14, 11, 7, 5, 3] (based on move_speed)
  • When anim_count reaches appropriate limit, either freezes there or increments anim_frame and goes back to 0. (see cases below)
  • when event animation_type is non_continous, remaining_step=0, and anim_frame=1 or 3, anim_count counts up to the non_continuous limit and then freezes there.
  • when event animation_type is non_continous, remaining_step=0, and anim_frame=0 or 2, anim_count counts up to the continuous limit and then anim_frame cycles to the next value (1 or 3)
  • when event animation_type is continous, remaining_step=0, anim_count counts up to the continuous limit and then anim_frame cycles to the next value
  • When remaining_step > 0, anim_count counts up to the non_continuous limit and then anim_frame cycles to the next value. Note that remaining_step decrements first, so this rule relies on the remaining_step value at the beginning of this frame.
  • Decreasing move_speed will cause anim_count to increment further as the limit increases.
  • Increasing move_speed will reduce the limit but anim_count will not change.
  • If you increase the move_speed enough that your current anim_count is now above the continuous limit for the new speed, the anim_frame will increment. #663
  • Setting a tile graphic doesn't change anything.
  • Change graphic move command has no effect on animations.
  • When jumping=1, then we force anim_count=0 and anim_frame=1. This means when a jump is started, the animation resets.
  • On the last frame of the jump, jumping=0 and anim_count=1. That is, the anim_count starts ticking on the last frame of the jump, instead of waiting for the next frame to start.
  • When no event page is active, anim_count=0 and anim_frame=1
  • SetEventLocation and SetVehicleLocation do not reset anim_count
  • Teleport resets anim_count
  • Is reset when anim_paused=1

Behaviors of anim_frame:

  • 0 = Left
  • 1 = Middle
  • 2 = Right
  • 3 = Middle
  • Defaults to 1, and then cycles 1, 2, 3, 0, 1, 2, 3, 0, ... as mentioned above in anim_count cases.
  • SetEventLocation and SetVehicleLocation do not reset anim_frame
  • Teleport resets anim_frame
  • Is reset when anim_paused=1

Animation of Vehicles:

  • Boat and Ship limit is always 15 regardless of move_speed and always continuously animating anim_frame to this.
  • Airship anim_count=0 and anim_frame=1 while aboard=0. This includes getting reset when landing
  • Airship limit is 11 and continuously animates while boarded
  • No change when player boards a vehicle, except the move speed of the vehicle (airship) can change how the animation continues as described above.
  • If you put any move route on the airship, it's anim_count limit changes to 15 instead of 11 and seems to stay there forever. If you quit and load and saved game it goes back to 11 until you make a new move route. RPG_RT bug?? ❗️

Behaviors of spinning events:

  • Spinning animation only changes sprite_direction.
  • A Turn move command changes both direction and sprite_direction
  • If a Turn move command is started on the same frame as animation, the direction will be of the turn, and sprite_direction = (direction + 1) % 4
  • A Move move command changes only direction
  • Triggering a spinning event via Action, Player Touch, or Event Touch does not affect direction or sprite_direction
  • When a jump is started, both direction and sprite_direction are set in the direction of the jump
  • When a jump is in progress, the spin will still continue to animate.
  • If a jump is started on the same frame as animation, the direction will be of the jump, and sprite_direction = (direction + 1) % 4

Player Controls:

  • Player controls don't respond until movement and stop_count of last move command finishes.

Vehicle Move Route interactions:

  • Airship move route gets canceled 1 frame after player boards
  • An active Boat, Ship move route gets canceled when player finishes boarding and aboard is set to 1.
  • Boat, Ship position fixed to player position when boarding finishes. This can be seen by moving the ship with a move route while the player is boarding.
  • If Player::vehicle > 0, SetMoveRoute against that vehicle gets applied to the player instead
  • If a move route gets set on the player while boarding, it will wait until the player finishes boarding and then execute on the player in the vehicle.

TBD:

  • How overwritten move route interacts with Event original move_route
  • Behavior of counters when move fails
  • Devirtualize GetOriginalMoveRouteIndex()
  • Don't store copy of original_move_route
  • When active move route is interrupted by another move route
  • How move route interacts with teleport
  • How move route interacts with get on/off vehicle #1429
  • Investigate through and route_through behavior when getting on/off vehicles
  • Effect of Erase Event and disabling all active pages. What happens when you do this mid move, stop_count, anim_count, etc...?
  • Effect of changing the active event page in the middle of a move or wait? Animations?
  • Effect of Animation Off move command during different animation frames.
  • Effect of failed jump on animation
  • Effect of wait for all movement
  • Effect of stop all movement
  • Effect of move_type=random

Please post more test cases if you can think of any not here.

@fmatthew5876 fmatthew5876 changed the title stop count refactors Move Route refactors Jan 5, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2019

@Ghabry

What is the story behind Game_Character::last_move_failed?

The only place we use this is in Game_Character::MoveTowardsPlayer() here:

https://github.com/EasyRPG/Player/blob/master/src/game_character.cpp#L495

Is there some known edge case where this logic is necessary? Otherwise it adds a lot of complexity and is incompatible with saved games. Why can't we recalculate the direction every frame regardless of move failure?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch 4 times, most recently from 36e0514 to a70c069 Jan 22, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

can't tell you the reason for "last_move_failed" yet but about "any_move_successful": Based on the comment in Game_player this was for fixing broken Ice physics in the game "Glimpse". Maybe you can try removing that handling and playing the game again maybe you fixed it in a better way by now.

And the other corner case is standing on a "on touch" handler which executes a skippable move command but that command fails, then the "on touch" handler doesn't fire again.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch from a70c069 to ce32321 Jan 24, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2019

This last_move_failed stuff seems not neccessary. I setup an event with move towards player with an obstacle in the way. As soon as I move the player around the event immediately changes direction to step towards the player.

Looks like it was added here:
eb90883

@Zegeri Do you remember why this logic was needed?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch from ce32321 to 6f1d2f8 Jan 25, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Jan 26, 2019

I've tested all of the cases from 1 to 14 in #1567. This PR matches RPG_RT with some exceptions

  • move_type == random has some wierd behavior that needs to be investigated further
  • There are some timing issues with the interpreter, such as player move routes being skipped for the very first frame, resulting in some off by 1 frame mismatches from the test cases. These are interpreter timing issues and not move route logic issues, so I will consider them separately.
  • I have not tested anim_count and anim_frame yet

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch 2 times, most recently from e5d1004 to 348bab9 Jan 26, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Jan 27, 2019

New changes, and now this PR passes test cases 15 and 16 from #1567.

Only caveat is again more instances of movement not being processed in the first frame.

@fmatthew5876 fmatthew5876 referenced this pull request Jan 27, 2019
3 of 3 tasks complete

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch from 348bab9 to 6148b26 Jan 27, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Jan 27, 2019

Fixed #1429

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2019

Now passes Player #1567 Test cases 17, 19 and 20.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch 3 times, most recently from f7ff595 to c6f44de Feb 4, 2019

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch from c6f44de to baff0bc Feb 5, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

This now passes #1567 Test cases 18

fmatthew5876 added some commits Jan 27, 2019

Allow the boat and ship to jump
Collision calculations for landing a jump did not take
into account terrain for boat and ship
Fix ship disembark behavior
Matches RPG_RT cases where player disembarks into the water, but
also puts the ship in the right place.

We do this for RPG_RT compatibility
Refactor event stepping animation
Fixing behaviors of anim_count and anim_frame to match RPG_RT
Fixes for behaviors when you talk to event
- Spinning events don't change direction : #1585
- Don't reset stop_count or any movement timers when you talk
Don't process player or vehicle movement on first frame of map
Matches RPG_RT behavior and fixes interpreter bugs in some games
Fix behavior of SetEventLocation and SetVehicleLocation
* SetVehicleLocation should reset remaining_step for vehicle
* Fix RPG_RT bug: Set*Location should reset the jumping flag
Animtype fixes
Fix behavior of direction and animation for different animation types
Devirtualize IsContinuous() and IsAnimated()

Passes #1567 Test 24
Fix behavior of boat/ship when teleporting while boarding
* Doesn't automatically board, waits for movement to finish
  in normal update loop.
* Boarding doesn't finish on first frame like other movement
Fix directions when finish boarding boat ship
* Direction is in direction of the player
* SpriteDirection of both player and vehicle set to Left
Refactor animation count
Redo the logic to make Player #663 and Test case 32 pass
Fix for SetAnimPaused and fixed_graphic
* SetAnimPaused(true) will reset anim frame
* Changing page to fixed_graphic will reset anim_frame
  but not anim_count
* SetAnimPaused() has no effect for vehicles
max_stop_count is always reset when event page changes
This even happens during an active move route, which can
cause strange effects (see Test cases 35 in #1567).
Behavior is preserved for RPG_RT compatibility

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_timing branch from 5b05ea2 to 1014b14 Mar 10, 2019

@carstene1ns carstene1ns merged commit e6c3cb4 into EasyRPG:master Mar 10, 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.