Opened 9 years ago

Closed 9 years ago

Last modified 7 months ago

#9143 closed patch

Zak C64 German: crash due to padding bytes

Reported by: SF/tobigun Owned by: sev-
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Zak McKracken

Description

The German version of zak c64 crashes when the intro (zak's dream) starts.
The crash is caused by a 0xFFFF padding word in room 47 that precedes the correct 16bit length of sound resource 95. The 0xFFFF is erroneously handled as the size of resource 95 and causes an overflow of reslen.
This 0xFFFF padding occurs only in the german version and only once in room 47.

Here the section of room 47
v= end of sound resource[1]
... FD 00 | FF FF | 1B 0B 00 04 ...
padding =^ ^= start of resource[95], first two bytes: size

Some more info:
- the index file (00.LFL) stores the correct index for resource 95 (room: 47, offset: 5596)
- the sizes of resource 95 according to the length field (value: 2843) are the same in the German and English version
- the 0xFFFF padding is at offset 5594
- the preceding resource (resource 1) starts at offset 4516. Its length field (which already includes the 2 bytes for the length field) has a value of 1078 (same as the English version)
-> 4516+1078=5594 points to the padding bytes and hence the 0xFFFF must be skipped

I already checked if my (original and uncracked) disk copy was corrupted or if there was some transmission error from my VC1541 to my computer but the disk image is correct.

The solution is very simple. The patch simply does not handle 0xFFFF (none of the LFL files is that large that a resource could be of that size) as a valid length and seeks for a non 0xFFFF value before handling the resource. The 0xFFFF is still written to the output, otherwise the 00.LFL offsets would not match.

The patch is just for ScummVM but the zak-c64 resource extractor has to be changed accordingly. It returns a 70KB sized 47.LFL (as it interprets 0xFFFF as resource length) which is not correct.

Ticket imported from: #2968610. Ticket imported from: patches/1248.

Attachments (1)

zak-c64-german.patch (557 bytes) - added by SF/tobigun 9 years ago.

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by SF/tobigun

Attachment: zak-c64-german.patch added

comment:1 Changed 9 years ago by fingolfin

I don't really know the C64 versions nor the code for them very well. But I wonder if this problem doesn't mean we are doing something wrong? In particular, it seems the code is based on the assumption that all resources follow after each other, with no gaps, and no alterations to the ordering.
In other SCUMM versions, there is a table of resource offsets somewhere, which contains for each resource at which offsets it starts. Is there such a table for the C64 version, too? If so, could it be used?
That would "better" than hacking around the broken assumption in our code.
On the other hand, since there aren't too many game variants affected by this, I guess in the end we can just do whatever results in the shortest / easiest code ;).

comment:2 Changed 9 years ago by SF/tobigun

The index file (00.LFL) contains a resource offset table with the resource start offsets relative to a room (virtual LFL file). This table is used by ScummVM whenever it loads a resource.
But before this can be done it extracts the room resource by ScummDiskImage::extractResource(room). Note that extractResource() does only extract a room (LFL file) resource and not a script, sound or costume resource.

As the table of resources does not contain the length of an LFL file, extractResource() has to determine it itself. This is done with the xyzResourcesPerFile tables and by iterating over the number of resources given there. This is where the described 0xFF padding bug occurs.

I already checked if it is possible to determine the last resource of a room, read its length and calculate the length with: size[room] = lastResOffset[room] + lastResLen[room]. The problem with this solution is that it does not work. The resource table contains resources that do not really exist, with offsets that are in the range of the next room. Probably those resources are never used and so the table entries just contain garbage. For example for zak the index table reports two resources that reside in 00.LFL but there are none. Even if you check the disk image, the data which should belong to those resources is just garbage. Even the 16bit size headers of those resources are invalid too.

It is possible to sort the room sector/track offsets and then extract the rooms according to this information. But this way almost all room resources would be followed by some garbage that should not belong to the room. So as long as the simple 0xFF fix works this might be the simplest solution.

comment:3 Changed 9 years ago by sev-

The patch looks fine to me, Thanks, committed as is.

comment:4 Changed 9 years ago by sev-

Owner: set to sev-
Status: newclosed

comment:5 Changed 7 months ago by digitall

Component: Engine: SCUMM
Game: Zak McKracken
Note: See TracTickets for help on using tickets.