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.

Sign up for GitHub

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

Jump to bottom

Refactor Interpreter command execution #1740

Merged
merged 24 commits into from
Aug 4, 2019
Merged

Refactor Interpreter command execution #1740

merged 24 commits into from
Aug 4, 2019

Conversation

mateofio
Copy link
Contributor

@mateofio mateofio commented Apr 6, 2019 •
edited
Loading

This PR modifies the interpreter to use SaveEventExecState and SaveEventExecFrame chunks so that it executes more accurately and allows correct saving and loading of LSD chunks while events are running.

Changes include

  • Refactoring call stack to use frame array instead of child interpreters
  • Fixing wait commands
  • Fixing key input proc
  • Fixing wait for all movement
  • Other cleanups

Control flow refactors using subcommand_path chunks will come in the next PR.

Depends: #1717
Depends: #1739
Depends: EasyRPG/liblcf#324

Fix: #757
Fix: #1777

Test Cases from #1707 addressed by this PR

  • 9
  • 10
  • 11
  • 12
  • 13 - commands work in rm2k3e mode
  • 13 - commands crash otherwise (We currently print a warning)
  • 14
  • 15
  • 16
  • 17
  • 18

@mateofio mateofio force-pushed the event_state branch 3 times, most recently from 79996f4 to 8bea0cd Compare April 6, 2019 13:11
This was referenced Apr 6, 2019
Closed
Closed
@mateofio mateofio force-pushed the event_state branch 13 times, most recently from 6292f5f to f2760a9 Compare April 7, 2019 17:25
@mateofio mateofio marked this pull request as ready for review April 7, 2019 17:29
@mateofio
Copy link
Contributor Author

mateofio commented Apr 7, 2019

I'm cutting this one here and will do the control flow / subcommand_path stuff in the next PR.

Test cases related to this have been listed and need to be verified.

@mateofio mateofio mentioned this pull request Apr 7, 2019
Merged
@fdelapena fdelapena added this to the 0.6.1 milestone Apr 11, 2019
@mateofio mateofio mentioned this pull request Apr 11, 2019
Closed
@mateofio
Copy link
Contributor Author

mateofio commented Apr 13, 2019 •
edited
Loading

All test cases pass, except that for doing "ThisEvent" from common events in Player causes warnings instead of crashing. Personally I think this should be fixed, as it quickly ends up in undefined/corrupted behavior but that falls under #1720 and I don't want a huge debate about it holding up this PR.

This is ready for review and merge.

@fdelapena
Copy link
Contributor

Diff between PR1717 and PR1740: mateofio/Player@scene_fixes...fmatthew5876:event_state

mateofio added 22 commits July 30, 2019 22:21
@mateofio
Remove Game_Map::ReserveInterpreterDeletion()
015ec32
@mateofio
Never delete Game_Event::interpreter
33920d8 
* Don't ever delete the interpreter, because allocating
and deleting is slow.
* Still don't allocate an interpreter until a parallel page
is activated, however.
* assert if we're parallel but there is no interpreter. This means
there is a bug that needs to be fixed.
@mateofio
Remove Game_Event::list
e5eaf83 
Don't make extra copies of the data. Also avoids potential
sync with page bugs.
@mateofio
Remove Game_Event::trigger
e5c5cc9 
Eliminates possibility of sync with page bugs
@mateofio
Remove Game_Event::from_save
70a8f7b 
This flag is only used during initiation and immediately cleared.
So pass it into Refresh() as a local variable instead of storing it.
@mateofio
Remove Game_Interpreter::list
b66862b 
Store an entire SaveEventExecFrame and use the
last element of its stack as the list.

Until we remove child interpreters, the stack articially
stays always as size 1.
@mateofio
Remove Game_Interpreter::index
eefa3cf
@mateofio
Use SaveEventExecFrame::triggered_by_decision_key
7805941
@mateofio
Remove Game_Interpreter::map_id
1502623 
Add Game_Interpreter_Map::OnMapChange() and call it for
the foreground interpreter.
@mateofio
Refactor event call stack
b5abe2c 
* Use SaveEventExecState::stack
* Remove child_interpreter
* Correct storage of event_id chunk
* Correct behavior for "ThisEvent" commands for rm2k3e and not
@mateofio
Fix wait commands
796a9e7 
* Use wait_time chunk. Allows frame accurate loading of save
  games with active wait commands.
* Waiting pan commands also use wait_time
@mateofio
Fix wait_for_key_enter behavior
276f006 
Use the correct chunk
@mateofio
Fix WaitForAllMovement execution
5d4ae42 
Use the wait_movement chunk and check in main update loop.
@mateofio
Refactor keyinputproc to use LSD chunks properly
4d18a40 
* Support rm2k and rm2k3 versions
* Abstract nasty details of chunk versioning
@mateofio
Implement RPG_RT call stack depth limit
8f714fc
@mateofio
Remove unecessary friend declaration
14543c7
@mateofio
Don't use floating point for _keyinput.wait_frames
5b5d8f8
@mateofio
Removed unused Game_Interpreter::updating
63dfab8
@mateofio
KeyInputProc: Handle Multiple key presses
bb428f9
@mateofio
Mention call stack limit value in logs
c1328f6
@mateofio
CallEvent / CallCommonEvent - handle empty events
4e7cfc6
@mateofio
KeyInputProc: Set var to 0 on first frame of wait
742421c
@mateofio
Copy link
Contributor Author

I found a regression in the game log.[in] (Download)

Go through the intro, start the computer, select "Enhanced Reality". Once entered you can't leave the ER menu anymore. That "Please turn on ER headset" doesn't always appear and that cancelling sometimes closes the whole PC window are game bugs.

This one is fixed now.

I have addressed all review comments and game regressions. Also rebased to master.

@Ghabry Ghabry mentioned this pull request Aug 4, 2019
Closed
8 tasks
@Ghabry
Copy link
Member

Ghabry commented Aug 4, 2019

Well the stuff I found before my vacation is fixed, so is fine for me now.
Will just merge this one now to get this finally in. But all the other PRs have to wait until next Sunday when I'm back home.

@Ghabry Ghabry merged commit 4c3913d into EasyRPG:master Aug 4, 2019
@mateofio mateofio mentioned this pull request Aug 29, 2019
Closed
6 tasks
@mateofio mateofio deleted the event_state branch December 29, 2019 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fdelapena fdelapena fdelapena approved these changes

@Ghabry Ghabry Ghabry approved these changes

Assignees
No one assigned
Labels
Event/Interpreter Refactor Savegames
Milestone
0.6.2
4 participants
@mateofio @fdelapena @Ghabry @carstene1ns