Refactor interpreter control flow #1742
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mateofio
force-pushed
the
event_cond
branch
4 times, most recently
from
02be291 to
504e1a0
Compare
mateofio
force-pushed
the
event_cond
branch
2 times, most recently
from
e2f1ede to
203261c
Compare
carstene1ns
added
the
Awaiting Rebase
fdelapena
removed
the
Awaiting Rebase
mateofio
force-pushed
the
event_cond
branch
5 times, most recently
from
8e57f21 to
13f9625
Compare
4 tasks
mateofio
force-pushed
the
event_cond
branch
3 times, most recently
from
5f06a72 to
00bb79d
Compare
fdelapena
approved these changes
Aug 19, 2019
|
This has been pretty heavily tested now as part of my work on #1876 . |
Ghabry
reviewed
Sep 1, 2019
src/game_interpreter_map.cpp
Outdated
| } | ||
|
|
||
| bool Game_Interpreter_Map::CommandDefeatHandler(RPG::EventCommand const& com) { // code 20712 | ||
| return CommandOptionGeneric(com, 2, {Cmd::EndBattle}); |
There was a problem hiding this comment.
Where do these numbers that are the 2nd parameter for CommandOptionGeneric come from?
There was a problem hiding this comment.
These are the values that subcommand_path.back() has to be to trigger this action.
There are example frame dumps in #1707
This deserves an enum to help make the code more self documenting.
Ghabry
reviewed
Sep 1, 2019
| frame = &_state.stack[current_frame_idx]; | ||
|
|
||
| // Only do auto increment if the command didn't manually | ||
| // change the index. |
There was a problem hiding this comment.
Under which cases does this happen?
There was a problem hiding this comment.
Any commands that need to manipulate the index internally. The show message is one complex example.
Loop commands are another
fdelapena
added
the
Awaiting Rebase
Ghabry
removed
the
Awaiting Rebase
4 tasks
After end loop, the next command is not Loop but instead is the first command inside the loop.
- Don't loop over choices or number input - Increment index after choices and number input (broken in earlier commit)
mateofio
force-pushed
the
event_cond
branch
from
ce06c7d to
62e286b
Compare
Ghabry
approved these changes
Sep 18, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #1740The PR refactors control flow to use
subcommand_pathchunks. The goal is to ensure that every event command is either executed or skipped the same asRPG_RT. The practical results of this are:Fixes most of #1707
Retested: #1613