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

SCUMM: (LOOM) - fix bugs #6011 and #13369 #3792

Merged
merged 2 commits into from May 12, 2022
Merged

Conversation

athrxx
Copy link
Member

@athrxx athrxx commented Apr 6, 2022

This fixes an old bug by adding some behavior of the original saveload script to our loading process (without actually running the script, since that would display the original save screen).

I turned this into a PR, so it can be thoroughly tested with as many savegames as possible.
The affected versions are DOS EGA, FM-TOWNS, Amiga and Atari ST.
I have tried loading many savegames for the versions I have, but there might be more.
And I don't have Atari ST.

(try to emulate the behavior of the save script without actually calling it)
engines/scumm/scumm.cpp Outdated Show resolved Hide resolved
engines/scumm/scumm.cpp Outdated Show resolved Hide resolved
engines/scumm/scumm.cpp Outdated Show resolved Hide resolved
@athrxx
Copy link
Member Author

@athrxx athrxx commented Apr 6, 2022

Thanks, I have updated the comments according to your suggestions.

@eriktorbjorn
Copy link
Member

@eriktorbjorn eriktorbjorn commented Apr 7, 2022

Unless I misunderstand completely, the bug still seems to be there with my English EGA version (1.1). Could this be another case where yours and mine use different variables?

I'd be happy to compare scripts. I've identified that script-4 is what sets variables 171-173, so that bit's probably the same. I'm not sure where the rest comes from, though.

@athrxx
Copy link
Member Author

@athrxx athrxx commented Apr 7, 2022

Thanks for testing. I do actually have two different EGA versions and it is true that they use different flags. But that should concern only whether the verb screen gets restored correctly.

But from your comment, you're probably talking about the bug with the flask? IIRC that should not depend on that. I was pretty sure it worked with both my EGA versions, with FM-Towns and with Amiga. If you do get your distaff restored than you do have a supported version.

Did you maybe save your game during with an active cut scene / disabled _userput? I should have made it clear that the fix will only work if you saved in a situation where you could have saved in the original. Otherwise ScummVM will just apply the same hack as before (a bit improved, but it will still not restore the flask). E. G. if the dripping from the flask is still in progress when you save, it cannot be restored.

@eriktorbjorn
Copy link
Member

@eriktorbjorn eriktorbjorn commented Apr 7, 2022

No, I thought this was about how if you save when an object (e.g. the leaf) is displayed in the lower right corner, and then reload that savegame, the object is not redrawn.

@eriktorbjorn
Copy link
Member

@eriktorbjorn eriktorbjorn commented Apr 7, 2022

I think I understand now. Without the patch, when I click on the object again it's not drawn. (Only the text is.) That does indeed seem to be fixed by this patch.

@athrxx
Copy link
Member Author

@athrxx athrxx commented Apr 7, 2022

Okay, so that's a misunderstanding then.

With the original, it will always clear the object image, too, right? This seems intentional and I didn't want to change it.

If people would want that image restored after loading (while still not having any of the bugs mentioned above) I guess it could be added as another candidate for _enableEnhancements. But preferably not in this PR

@eriktorbjorn
Copy link
Member

@eriktorbjorn eriktorbjorn commented Apr 7, 2022

With the original, it will always clear the object image, too, right? This seems intentional and I didn't want to change it.

Yeah, it seems to behave like the original. I just misremembered what it was about.

@dwatteau
Copy link
Contributor

@dwatteau dwatteau commented Apr 20, 2022

I've tested this with my French EGA version (incl. some 2006 savegames) and everything looked fine 👍

@athrxx
Copy link
Member Author

@athrxx athrxx commented Apr 20, 2022

Thanks for testing! I'll leave this open for a bit longer, since there is no particular hurry (the bug ticket is 10 years old and these are just minor glitches).

@athrxx
Copy link
Member Author

@athrxx athrxx commented May 12, 2022

Okay, this can really go in now...

@athrxx athrxx merged commit f66aad4 into scummvm:master May 12, 2022
8 checks passed
@athrxx athrxx deleted the scumm-saveload branch May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants