SCUMM: Detection of Mac version of MI1 from the LucasArts Mac CD Game Pack is broken when using Dumper Companion

[eriktorbjorn]

To verify that the Mac versions of the SCUMM games still work, I re-dumped the files using the ScummVM dumper companion. I tried all the games from the two Mac CD Game Packs, as well as my Fate of Atlantis CD. They were all detected, except for Monkey Island 1 from the first game pack.

I tried dumping both with and without Unicode support, but that doesn't make any difference for this particular game. Both yield the following files:

• MONKEY1.000
• MONKEY2.000
• MONKEY3.000
• MONKEY4.000
• MONKEY5.000
• Monkey Island

The game gets detected as any of:

• The Secret of Monkey Island (CD)
• The Secret of Monkey Island (Mac)
• The Secret of Monkey Island (FM-TOWNS/FM-TOWNS)
• The Secret of Monkey Island (SEGA/SegaCD)
• The Secret of Monkey Island (SE Talkie)

I picked the Mac one, but there's obviously something wrong here because when I run it it first complains that the game should have audio tracks (it doesn't), and then it crashes with a "Bad ID 69622426('ib$&') found in index file directory!" error.

This is probably a regression from when I tried to fix the detection entries. Interestingly, the one from the second Mac Games pack (which has only MONKEY1.000, MONKEY1.001, and Monkey Island) is unambiguously detected and works fine.

It's probably some minor mistake I made. But even if I find the time to look, I won't be able to commit anything until this evening at the earliest. And it's a pretty bad bug to have at this stage, so I'm filing a bug report in case anyone else wants to look at it before I can.

Is this something that needs addressing before 2.9.0?

[eriktorbjorn]

Wait a minute...

For all the games, Dumper Companion extracts the data files (MONKEY1.000, etc.) as raw data.

Except for Monkey Island on the first pack, where the files get extracted as MacBinary (creator 'Mky1', type '.LFL').

So apparently the files have a small resource fork? That's awkward. If I re-copy the MONKEY1.* files as raw data, the game detects and runs just fine. I'm not sure how to handle this.

[eriktorbjorn]

Judging by ResEdit, the resource forks for these files are actually empty. According to hfsutils the resource forks are 286 bytes each, but maybe that's just the overhead of having a resource fork to begin with?

[sev-]

The proper way of handling it is to use the MacResource class for accessing these files. That is written in a way that it should be format-agnostic. So far, we are supporting seven formats, and there is yet AppleOne, which we do not support, and maybe never support, as it was used in a relatively short timespan and was replaced by AppleDouble.

For the SCUMM engine, it could be two-fold. Besides accessing the files in the engine itself, its custom detection code may also attempt to compute MD5s for the first 5000 bytes as is, but in MacBinary format, the first 128 bytes are the header, which is mutable.

ResEdit may report 0 bytes of resource fork because resource forks have their own structure. They contain a smaller header, an index of resources, and the so-called "Data Part," which typically is a concatenated set of resources, though I do not exclude padding in between. In the AdvancedDetector, we compute the MD5 of this data portion, not from the beginning of the resource fork.

[eriktorbjorn]

After looking some more, it seems that the SCUMM engine will already transparently handle MacBinary files (see ScummFile::open()), and the game detection also tries to (see part 1 of detectGames()).

Yet the detection fails.

I think the problem is that it tries to verify that it's a MacBinary file from whatever position the stream is left at after calculating the MD5 sum. If I first rewind it to the beginning, it seems to work fine:

diff --git a/engines/scumm/detection_internal.h b/engines/scumm/detection_internal.h
index 6c0c189d47c..c4aa0ba344d 100644
--- a/engines/scumm/detection_internal.h
+++ b/engines/scumm/detection_internal.h
@@ -533,6 +533,7 @@ static void detectGames(const Common::FSList &fslist, Common::List<DetectorResul
                                d.md5Entry = findInMD5Table(md5str.c_str());

                                if (!d.md5Entry && (platform == Common::Platform::kPlatformMacintosh || platform == Common::Platform::kPlatformUnknown)) {
+                                       tmp->seek(0);
                                        Common::SeekableReadStream *dataStream = Common::MacResManager::openDataForkFromMacBinary(tmp);
                                        if (dataStream) {
                                                Common::String dataMD5 = computeStreamMD5AsString(*dataStream, kMD5FileSizeLimit);

Does that seem like a sensible fix?

[Torbjörn Andersson <eriktorbjorn@…>]

In c86e1322:

SCUMM: Fix detection of some (one?) Mac versions (bug #15509)

The version of Monkey Island 1 included on the LucasArts Mac CD Game
Pack has, for reasons unknown, empty resource forks on the data files.
When using the Dumper Companion it would create MacBinary files for
them, which the detector didn't recognize.

This was because of a bug in the detector, where it didn't rewind the
stream before determining if it was a MacBinary file. (The SCUMM engine
already handles MacBinary files transparently, so no changes were needed
there.)