Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#3458 closed defect (fixed)

MM: Walk behind mask char data

Reported by: (none) Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
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 11 years ago.
Log of output.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by fingolfin

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

comment:2 Changed 11 years ago by fingolfin

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 Changed 11 years ago by (none)

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 Changed 11 years ago by fingolfin

Any comments on this, aquadran?

comment:5 Changed 11 years ago by fingolfin

Owner: set to aquadran

comment:6 Changed 11 years ago by aquadran

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 Changed 11 years ago by fingolfin

Owner: changed from aquadran to sev-

comment:8 Changed 11 years ago by fingolfin

In which room(s) did you observe this?

Changed 11 years ago by SF/*anonymous

Attachment: mm.log added

Log of output.

comment:9 Changed 11 years ago by (none)

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 Changed 11 years ago by fingolfin

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 Changed 11 years ago by fingolfin

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