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

Save game and move route fixes #1580

Merged
merged 4 commits into from Dec 20, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Dec 17, 2018

Fixes: #1571

I tested and this also correctly loads the mountain background from Razas save game from #1498

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:pic_fix branch from 44a34ba to 5d929f5 Dec 18, 2018

data.flash_green = 0;
data.flash_blue = 0;
data.flash_time_left = 0;
data.flash_current_level = 0;
flash_sat = 0;
flash_period = 0;

This comment has been minimized.

@fmatthew5876

fmatthew5876 Dec 18, 2018

Author Contributor

Below this line (you can't see it unless you expand), we reset the tint if its negative.

Is it actually the case a negative value ever gets written to the tint chunks? If so it would be better to do this in fixup in liblcf.

@fmatthew5876 fmatthew5876 changed the title Don't erase all pictures when loading saved game Some load game cleanup fixes Dec 18, 2018

@fdelapena fdelapena added the Savegames label Dec 18, 2018

@fdelapena fdelapena added this to the 0.6.0 (likely) milestone Dec 18, 2018

fmatthew5876 added some commits Dec 17, 2018

Refactor Game_Screen::Reset()
Fixes: #1571

* Call before loading savegame to reset state.
* Don't call after savegame loaded - don't erase pictures or reset state
Refactor Game_Map::Dispose()
* Call before loading save game
Fix battle regression
Broke battle screen transition and after battle cleanup

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:pic_fix branch 2 times, most recently from 7345419 to 12282b2 Dec 19, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 19, 2018

Added a battle regression fix and a brand new vehicle bug fix

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:pic_fix branch from 12282b2 to 10ebac7 Dec 19, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 19, 2018

There was another bug in HH3 where player movement followed by ProceedWithMovement would fail.

This last one has to do with ProceedWithMovement commands. Why do we only check for IsMovementRepeated() in Game_Map::IsAnyMovePending()?

This makes no sense, and I can't imagine how ProceedWithMovement ever worked. It looks like Game_Map::IsAnyMovePending() hasn't been updated in 4 years??

@Ghrabry Please help!! I'm very confused about this one. What are we supposed to be doing in Game_Map::IsAnyMovePending()???

😕

@fmatthew5876 fmatthew5876 changed the title Some load game cleanup fixes Save game and move route fixes Dec 19, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Dec 19, 2018

I don't get it. In the second to last commit you remove the ! from the if and in the commit after you conclude that this change is broken and delete it???

And the "IsRepeated" check is easy to explain: Any event move route is registred in the pending move list but from a "Proceed with movement" point of view a move route is considered finished when it was executed fully once (there is an option to make a Move Route repeat).

That implementation is corret, otherwise hundreds of games would break.

Proceed with movement vehicle fix
If you try to move a vehicle which isn't on the map,
ProceedWithMovement will not wait for this to complete.
Instead the movement stays queued forever until you enter
the same map as the vehicle.

This also works if you use SetVehicleLocation() after the
move command to put the vehicle on the same map as you.

Once the vehicle is on the same map as the player, the movement
will immediately execute.

Fixes an issue where the game would hang in HH3 when
ProceedWithMovement was used.

To see the bug before this commit, setup the following event code

```
Move Vehicle Airship: Up
ProceedWithMovement
Teleport to map with airship
```

In RPG_RT you'll teleport, and then upon arriving the airship
will move. In player the game will hang.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:pic_fix branch from 973f54b to acbc0d1 Dec 19, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 19, 2018

Well thats what happens when you are writing code all night..

I fixed this. My test case works now, with the vehicle fix.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Dec 19, 2018

I will make some tests concerning Moves that cross map boundaries

@@ -3076,9 +3076,6 @@ bool Game_Interpreter::DefaultContinuation(RPG::EventCommand const& /* com */) {

void Game_Interpreter::ResetEventCalling() {
event_calling = {};
// FIXME: Need to separate immediate battle calls from events
// and SavePartyLocation::encounter_calling for random encounters.
Game_Temp::battle_calling = false;

This comment has been minimized.

@Ghabry

Ghabry Dec 19, 2018

Member

I don't see any other change related to battle calling. Is just deleting this really the solution?

This comment has been minimized.

@fmatthew5876

fmatthew5876 Dec 19, 2018

Author Contributor

This is a regression from f4439f2.

The clearing of battle_calling was not supposed to be added there. In master, the flag is cleared before the battle starts, which disabled battle clean logicup like BGM reset from being triggered after the battle.

The flag has always been properly cleared in scene_map.cpp after the battle ends. I don't like throwing global flags around like this, but refactoring it is another PR.

@Ghabry

Ghabry approved these changes Dec 19, 2018

Copy link
Member

Ghabry left a comment

Made some tests involving proceed movement and teleport in/out vehicle and vehicles on different maps: It fixes the one mentioned issue and doesn't break the others.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Dec 20, 2018

actually the load scene hang is gone in master, maybe got the wrong download from Jenkins. Also cherry-picking your stuff doesn't add the hang.

@carstene1ns carstene1ns merged commit 76b12d9 into EasyRPG:master Dec 20, 2018

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:pic_fix branch Jan 29, 2019

@Ghabry Ghabry added the Move Routes label Feb 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.