Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#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)

mm.log (9.0 KB ) - added by SF/*anonymous 17 years ago.
Log of output.

Download all attachments as: .zip

Change History (12)

comment:1 by fingolfin, 17 years ago

Summary: Walk behind mask char dataMM: Walk behind mask char data

comment:2 by fingolfin, 17 years ago

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? :)

comment:3 by (none), 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:4 by fingolfin, 17 years ago

Any comments on this, aquadran?

comment:5 by fingolfin, 17 years ago

Owner: set to aquadran

comment:6 by aquadran, 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 fingolfin, 17 years ago

Owner: changed from aquadran to sev-

comment:8 by fingolfin, 17 years ago

In which room(s) did you observe this?

by SF/*anonymous, 17 years ago

Attachment: mm.log added

Log of output.

comment:9 by (none), 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 fingolfin, 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 fingolfin, 17 years ago

Owner: changed from sev- to fingolfin
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.