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

ScummTR corrupts MONKEY1-VGA "v1.0" in ScummVM #47

Open
1 task done
dwatteau opened this issue Feb 9, 2022 · 6 comments
Open
1 task done

ScummTR corrupts MONKEY1-VGA "v1.0" in ScummVM #47

dwatteau opened this issue Feb 9, 2022 · 6 comments
Assignees
Labels
bug Something isn't working game corruption One of the ScummTR tools corrupts a game

Comments

@dwatteau
Copy link
Owner

dwatteau commented Feb 9, 2022

Summary

Commit dc94b2a added a workaround for the following duplicate offset in MONKEY1-VGA-FR:

ERROR: Duplicate offset in index: 0x18E82 in room 59

Indeed, the commit above let one use ScummRP/ScummTR on MONKEY1-VGA-FR (although I'm not completely sure we're removing the right duplicate yet, see issue #31), but doing so corrupts the game in ScummVM:

scummtr -g monkey -of tmp.txt
scummtr -g monkey -if tmp.txt

# Or:
scummrp -g monkey -od DUMP
scummrp -g monkey -id DUMP

image

In DOSBox, this screen is OK, but ScummVM tends to be more robust than the original interpreter, so we're probably really corrupting something (I haven't done a full game play yet).

I can't compare this with the original scummtr.exe 0.4.0 release, because this version had the Duplicate offset in index error above. But, recompiling ScummTR 0.4.1 with commit dc94b2a on top of it in Visual C++ 2005 Express (which should be quite close to an original release) has the same behavior, so I don't think that it's because of a recent regression.

This looks quite similar to issue #45, but I do have the std::stable_sort fix, and using an older compiler doesn't seem to change anything.

Impacted games

The Secret of Monkey Island

ScummTR versions

Current Git HEAD

Relevant output (error messages, warnings, or any useful info)

No response

I own a legitimate game

  • I promise that my game does NOT come from an illegal source, such as "abandonware" websites
@dwatteau dwatteau added the bug Something isn't working label Feb 9, 2022
@dwatteau dwatteau self-assigned this Feb 9, 2022
@dwatteau dwatteau pinned this issue Feb 9, 2022
dwatteau added a commit that referenced this issue Feb 9, 2022
…tion bug

Some versions of MONKEY1-FLOPPY-VGA (possibly only the earliest 1.0
version) have a duplicate 117/10 costume inside room 59 (Stan's).

Commit dc94b2a tried to ignore the
duplicate, as is already done for duplicate scripts, but it looks like
this doesn't work as expected for duplicate costumes, because ScumMVM
just glitches when displaying the modified game (as 000.LFL appears
to contain some invalid bytes).

I couldn't find an easy fix for this, and it's probably too late to
spend more time on this for the 0.5.0 release, so let's just reject
this game variant for now (earlier releases didn't support it at all).

Issue #47.
@dwatteau
Copy link
Owner Author

dwatteau commented Feb 9, 2022

Comparing the game data files before a simple scummrp reimport show that only 000.LFL is modified, in 4 bytes:

--- 000.LFL.original
+++ 000.LFL.modified
@@ -219,7 +219,7 @@
 00000f20  00 00 00 00 00 00 00 3c  c2 67 00 00 0b 67 d9 00  |.......<.g...g..|
 00000f30  00 2d 84 79 00 00 0e 34  82 00 00 0a bb ae 01 00  |.-.y...4........|
 00000f40  0b a1 de 00 00 0a 9f b0  01 00 3a a4 83 01 00 2d  |..........:....-|
-00000f50  7a db 00 00 55 97 e0 00  00 3b[82 8e 01 00]3b d8  |z...U....;....;.|
+00000f50  7a db 00 00 55 97 e0 00  00 3b[ff ff ff ff]3b d8  |z...U....;....;.|
 00000f60  9b 01 00 25 e0 42 01 00  25 8e 58 01 00 35 14 2b  |...%.B..%.X..5.+|
 00000f70  01 00 35 11 49 01 00 35  84 1c 01 00 35 2f 25 01  |..5.I..5....5/%.|
 00000f80  00 3b bb fc 01 00 00 00  00 00 00 00 00 00 00 00  |.;..............|

The ff ff ff ff probably come from setting the offset to -1 in the workaround. I don't know if it's a valid value there; I doubt it, although the DOS version does appear to tolerate it (but I haven't played a full game to confirm it; the duplicate costume is only triggered at the very end of the game, when LeChuck sends you through the grog machine).

It looks like ScummRp may have a special treatment when an offset is set to -1 for duplicate scripts, but maybe something is wrong/missing when dealing with duplicate costumes.

By the way, my French version (with the bug) shows Version 1.0 in Ctrl-V, while my English version (without the duplicate costume) shows Version 1.1. So this problem is maybe just limited to the earliest VGA floppy versions, but we'd still like to have a proper workaround for them, as as far as I know there's no 1.1 French FLOPPY-VGA version.

Anyway, I've pushed commit 4f92fe2, so that ScummTr 0.5.0 just rejects this version, instead of corrupting the game. I just don't know how to fix this at the moment, and I'd like to release ScummTR 0.5.0 this month. Since no earlier version of ScummTr had support for these variants anyway, it's not going to be a regression…

@dwatteau dwatteau added the game corruption One of the ScummTR tools corrupts a game label Feb 21, 2022
@dwatteau dwatteau changed the title ScummTR corrupts MONKEY1-VGA-FR in ScummVM ScummTR corrupts MONKEY1-VGA "v1.0" in ScummVM Feb 26, 2022
@dwatteau
Copy link
Owner Author

Looks like this bug may exist in all "v1.0" versions of the VGA game:

https://forums.scummvm.org/viewtopic.php?p=89717#p89717

@dwatteau
Copy link
Owner Author

dwatteau commented May 4, 2022

I actually wonder if this corruption doesn't just come from the parsing problem which causes #54 and #56

EDIT: If you unpack/repack MONKEY1-FLOPPY-VGA with ScummPacker, the same glitch is triggered.

@dwatteau
Copy link
Owner Author

dwatteau commented Oct 18, 2022

FWIW, regarding the "glitch" above, if the issue only shows up in ScummVM, that's maybe because the 000.LFL index file gets changed a bit, and so ScummVM won't recognize it as Monkey1 VGA but Monkey1 EGA (which is wrong), since its fallback detection code is currently a bit limited.

So actually ScummPacker's fix for the index problem in Monkey1 VGA is maybe OK, but the ScummVM detection bug just made me misdiagnose the issue.

@gabberhead
Copy link

gabberhead commented Oct 27, 2024

i have the same problem with the german vga dos version. dosbox is working, scummvm gives the same glitches. i try to maake a re-translation of the game. but it can not be played with scummvm because of that. the german ega version is working fine :)

@dwatteau
Copy link
Owner Author

Hmm yeah that's maybe another reason why improving the fallback detection code in ScummVM's engines/scumm/detection_internal.h would be helpful.

I don't think there's much interest from the rest of the ScummVM test in working on this improvement, but I do see the cases where it could benefit from it.

And since there are several reasons for me to do this, it increases its priority in my todo-list…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working game corruption One of the ScummTR tools corrupts a game
Projects
None yet
Development

No branches or pull requests

2 participants