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

Improve order of event updates #753

Merged
merged 9 commits into from Feb 8, 2016

Conversation

Projects
None yet
2 participants
@Zegeri
Member

Zegeri commented Feb 6, 2016

The most important bit is in Game_Map::Update. The process in which all the events are updated is now more convoluted, but also more accurate to the output of the event tracer plugin. To make the code more digestible, I've changed the container of the events and common events from std::map<int, EASYRPG_SHARED_PTR<Game_(Common)Event>> to std::vector<Game_(Common)Event>. This means a linear search, but we don't ever search common events and there are few maps with more than a thousand events.

Some facts about the game logic update thanks to the event tracer.

  1. The order of event update is: Parallel common event, parallel event, non-parallel event, non-parallel common event.
  2. A keypress starts an event in the same frame it gets detected by KeyInputProc command.
  3. If a parallel (common) event pushes a move route on the hero, it will start in that frame. If it's a non-parallel (common) event, the route starts in the next frame.
  4. The hero move route is executed between the update of parallel events and non-parallel events.
  5. The hero won't move or trigger other events by keypresses, if a non-parallel event will execute commands in that frame.
  6. If a non-parallel event needs more than one frame to be executed, it'll run before any other non-parallel event in the next frame.

From (2), (3) and (4), I've concluded that the hero has to be updated before the non-parallel events, but it must be aware if there's any auto-starting or collision-triggered event because of (5). That's what the workaround function Game_Map::IsAnyEventStarting is for.

  • Fixes #692
  • Fixes the combat system of Violated Heroine: The monsters do actually attack the heroine now
  • Fixes being stuck speaking with Marin in Zelda's Link Awakening
  • Probably more fixes in other interpreter intensive games
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Feb 7, 2016

Member

This also fixes #677
And the coin counter in "itwr" after loading a savegame.

Well done. For analyzing when move routes are updated doing asm patches via dynrpg was really worth it.

Very cool and crazy (wtf did Enterbrain think when inventing this) at the same time.

Member

Ghabry commented Feb 7, 2016

This also fixes #677
And the coin counter in "itwr" after loading a savegame.

Well done. For analyzing when move routes are updated doing asm patches via dynrpg was really worth it.

Very cool and crazy (wtf did Enterbrain think when inventing this) at the same time.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Feb 7, 2016

Member

Is that PR a proper fix for #626 ?

Needs testing: #662 and #703

Member

Ghabry commented Feb 7, 2016

Is that PR a proper fix for #626 ?

Needs testing: #662 and #703

@Zegeri

This comment has been minimized.

Show comment
Hide comment
@Zegeri

Zegeri Feb 7, 2016

Member

This still needs the workaround Game_Player::IsBlockedByMoveRoute, so do not close #626 yet.

Member

Zegeri commented Feb 7, 2016

This still needs the workaround Game_Player::IsBlockedByMoveRoute, so do not close #626 yet.

@Zegeri

This comment has been minimized.

Show comment
Hide comment
@Zegeri

Zegeri Feb 8, 2016

Member

Open for revision. I'm not going to tackle more issues for now.

Member

Zegeri commented Feb 8, 2016

Open for revision. I'm not going to tackle more issues for now.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Feb 8, 2016

What is the purpose of that loop?
It loops up to 500 times when the event is finishing in one update tick but why the 500?

Ghabry commented on src/game_commonevent.cpp in 341b4c4 Feb 8, 2016

What is the purpose of that loop?
It loops up to 500 times when the event is finishing in one update tick but why the 500?

This comment has been minimized.

Show comment
Hide comment
@Zegeri

Zegeri Feb 8, 2016

Owner

It's the actual behaviour of autostarting common event, they loop and run multiple times in each frame while their conditions are met. In RPG_RT, the number of loops depends on how many event commands it has. The more event commands it has, the less it loops. I think 500 loops is good enough.

Owner

Zegeri replied Feb 8, 2016

It's the actual behaviour of autostarting common event, they loop and run multiple times in each frame while their conditions are met. In RPG_RT, the number of loops depends on how many event commands it has. The more event commands it has, the less it loops. I think 500 loops is good enough.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Feb 8, 2016

Member

This also fixes a hang in Vampires Dawn 2 that got reported via Android Bug reporting.

Member

Ghabry commented Feb 8, 2016

This also fixes a hang in Vampires Dawn 2 that got reported via Android Bug reporting.

Ghabry added a commit that referenced this pull request Feb 8, 2016

Merge pull request #753 from Zegeri/development
Improve order of event updates

@Ghabry Ghabry merged commit 64df48c into EasyRPG:master Feb 8, 2016

5 checks passed

Android Build finished. No test results found.
Details
Linux Build finished. No test results found.
Details
OSX Build finished. No test results found.
Details
Windows Build finished. No test results found.
Details
web Build finished. No test results found.
Details

Zegeri added a commit to Zegeri/Player that referenced this pull request Jul 12, 2016

Remove Game_Interpreter::HasRunned; it's no longer needed
Now that all non-parallel events are executed after the player update
(since pull #753) and we don't need the flag 'runned' while auto-starting
events restart (since previous commit), this doesn't make any meaningful
difference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment