#3458 closed defect (fixed)
MM: Walk behind mask char data
| Reported by: | (none) | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: | Maniac Mansion |
Description
Nothing serious I hope and feel guilty about posting it really. I was mucking around with the LFL files for the V1 Maniac Mansion with the help of the documentation on Lucas Hacks. The document stops short at explaining how the Walk behind mask char data is stored (compressed, how big etc) so anyway I had a look at the
void GdiV1::roomChanged(byte *roomptr) method in gfx.cpp and in particular the line
decodeC64Gfx(roomptr + READ_LE_UINT16(roomptr + 18) + 2, _C64.maskChar, READ_LE_UINT16(roomptr + READ_LE_UINT16(roomptr + 18)));
My code is as below
// Mask Char Map (not sure that this works, sizes don't seem to match up)
offset = (fileData[19] & 0xFF)<<8; offset |= fileData[18] & 0xFF;
int size = (fileData[offset+1] & 0xFF)<<8; size |= (fileData[offset] & 0xFF); size -= 8; // MANIAC MANSION V1/DOS
byte[] maskChar = new byte[size];
Decompress.decompress(fileData, offset+2, maskChar);
The decompress method does the same job as decode C64Gfx.
The sizes of the Walk behind char map regions in the LFL files I have are stated as being 8 bytes larger than they actually are. Nothing serious but in Java that causes and ArrayOutOfBounds exception.
That's it. The only other thing to note is that I got the LFL files by way of the Macintosh version of DOTT.
Sorry.
Ticket imported from: #1837375. Ticket imported from: bugs/3458.
Attachments (1)
Change History (12)
comment:1 by , 17 years ago
| Summary: | Walk behind mask char data → MM: Walk behind mask char data |
|---|
comment:2 by , 17 years ago
comment:3 by , 17 years ago
I *think* I've found an out-of-bounds in ScummVM.
As far as the English V1 Maniac Mansion LFL files taken from the Mac version of DOTT is concerned the line
decodeC64Gfx(roomptr + READ_LE_UINT16(roomptr + 18) + 2, _C64.maskChar, READ_LE_UINT16(roomptr + READ_LE_UINT16(roomptr + 18)));
should perhaps be
decodeC64Gfx(roomptr + READ_LE_UINT16(roomptr + 18) + 2, _C64.maskChar, READ_LE_UINT16(roomptr + READ_LE_UINT16(roomptr + 18))-8);
The worst that's happening with the SVN code as of now is that it's writing garbage at the end of _C64.maskChar, and it might go out of bounds. I can't say if the same is true for the other V1 game files.
Hope that helps clarify my meaning.
comment:5 by , 17 years ago
| Owner: | set to |
|---|
comment:6 by , 17 years ago
That part of code was based on sources of scumm16 tool. I never analysed that part if it's ok or not. So you are propably right about ArrayOutOfBounds and code need to be fixed with changing size of 8 bytes. But first that need to be checked if this happen always in the game and with other v1 games.
comment:7 by , 17 years ago
| Owner: | changed from to |
|---|
comment:9 by , 17 years ago
All the rooms. As far as I'm aware.
I've attached a file 'mm.log' which contains the following entry for all the rooms
================================== Processing 03.lfl java Maniac ../../games/mm/03.lfl Max mask value : 3 Size of mask char map required is 4* 8 = 32 Short at offset 18 : 40 ==================================
It prints out the maximum 'character number' used by the mask map. Then the number of bytes needed to store the character data for the mask and then it prints out the short value at offset 18 in the file, this apparently being the size of the mask character data. It's always 8 out.
If you like I can attach my Java tool set, but the code wasn't really intended for anything but domestic consumption.
File Added: mm.log
comment:10 by , 17 years ago
Yes, you are completly right about this. I just verified that in our current code, decodeC64Gfx will produce more data than what we ask it for; but if I subtract 8, as you suggest, then the value "x" and "size" match at the end of decodeC64Gfx().
So, I just commited the fix together with a comment pointing here. Thanks a lot for pointing this out!
comment:11 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
Hm, I am not quite sure what you want to tell us here. Did you find an out-of-bounds in ScummVM? If so, where precisely? Or are you just saying that some docs you found out there are incorrect? Or what else is your message? :)