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 Teleport #1694

Merged
merged 17 commits into from Apr 14, 2019

Conversation

@fmatthew5876
Copy link
Contributor

commented Mar 12, 2019

Depends on #1687

Fix: #1624
Fix: #1693
Fix: #1316
Fix: #1567

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

This passes all the test cases and is ready for review.

Now some trivia. If you MoveToVariableLocation to (-1,8) in RPG_RT on horizontal looping map, your position will look correct (as if you're at x=10) but the position_x chunk has the value of -1. ❗️ Step left or right and x will fix itself to the correct looped value.

Not sure what kind of bugs that might cause with collisions in RPG_Rt.

This RPG_RT bug is fixed in Player.

@carstene1ns

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

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

@fmatthew5876 fmatthew5876 referenced this pull request Mar 13, 2019
1 of 2 tasks complete

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_teleport branch 3 times, most recently from 0655a1f to fe2ff04 Mar 13, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

This causes multiple issues:

https://easyrpg.org/play/pr1694/?game=yume_nikki
After the instructions the screen stays black.

https://easyrpg.org/play/pr1694/
Talk to the dude next to the bridge, the message box closes after the teleport.

@Ghabry
Copy link
Member

left a comment

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_teleport branch 2 times, most recently from 3f5ff66 to e329dc1 Mar 26, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

I fixed the test game issue, but the Yume Nikki thing is still outstanding.

It only happens on web, and not on my local build in linux. The first "Refactor Teleport" commit moved things around and touched some code with comments about emscripten, so this is probably a web only regression.

@Ghabry What exactly is the issue here with async loading and transitions that breaks emscripten?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_teleport branch 7 times, most recently from b67ea0c to d76cbf6 Mar 27, 2019

@Ghabry

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

woah the yume nikki fadein is for the first time looking correct. great.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_teleport branch 3 times, most recently from 05cb085 to 1d0b4bb Mar 29, 2019

fmatthew5876 added some commits Mar 12, 2019

Refactor teleport
* Correctly handle PreUpdate() routine from teleport
* Scene calls at the same frame as teleport are deferred until teleports
are done.
* Remove Game_Map::IsTeleportDelayed() concept - not needed with
PreUpdate()
* Allow multiple teleport commands each frame - last one wins
* Don't async load until we actually start the teleport. To avoid
  issues with multiple teleport commands.
* Add error check for map_id <= 0 - crashes like RPG_RT
* Use OnTransitionFinish() to handle finishing teleport
Don't reset processed flag on preupdate
Benign, but matches RPG_RT behavior
Fix transition in for Escape / Teleport spell
When these skills are used, we always do a slow fade in,
regardless of the configured transition for teleport.
Game_Player::MoveTo() don't clamp position
RPG_RT allows you to teleport to negative coordinates with variables
if you so choose.
Don't process teleport commands if message is visible
Fixes regression where the game would teleport while the
message box was animating closed.
Scene Map: Refactor async handling
Fix teleport logic to handle async loading and fix
emscripten

* Replace OnTransitionFinish() with a generic continuation
* Add IsAsyncPending() wrapper
* Reshuffle the logic to make all this work
AsyncHandler: Add IsGraphicFile() concept
Any graphical asset gets this flag.
Handle event requested transitions on async platforms
Refactor Scene_Map::Update() to check for async conditions
after Game_Map::Update() and then set a continuation to
continue the update loop and process transitions.

Event requests transitions are disabled in PreUpdate() for now.
That requires more research and already likely triggers
RPG_RT bugs.
Fix Async Handling and the frame counter
If the Update() loop calls an async operation, defer incrementing
the frame counter until all the async ops are complete.
Fix Escape / Teleport skill
This broke when we removed OnTransitionFinish()

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:event_teleport branch from 1d0b4bb to fe130f1 Apr 7, 2019

@Ghabry

Ghabry approved these changes Apr 14, 2019

Copy link
Member

left a comment

Well due to lack of Async support in C++ this is again kinda ugly but my tests confirm that it works.

src/async_handler.h Outdated Show resolved Hide resolved
src/teleport_target.h Outdated Show resolved Hide resolved
@Ghabry

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

Tested #1416: Works.

Log.[in] seems to be broken in the web player in general (in all branches), not tested there.

Edit: Local fake-async works though

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

I fixed the style comments

@carstene1ns

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

ok

@carstene1ns carstene1ns merged commit dea4f9c into EasyRPG:master Apr 14, 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

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:event_teleport branch Apr 16, 2019

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.