Opened 12 years ago

Closed 12 years ago

Last modified 2 years ago

#8911 closed patch

CRUISE: 64bits fixes

Reported by: bgK Owned by: sev-
Priority: normal Component: Engine: Cruise
Keywords: Cc:
Game:

Description

The Cruise for a corpse engine did not run on 64bits machines mainly due to loading data directly to structures and bad pointer manipulations (bug 1848957).

The attached patch fixes loading the overlay structures, and the construction of the polygon list. I haven't done extensive testing, so the 64 bits version is probably not yet on par with the 32 bits one. But at least the intro is now displayed correctly and the first few scenes are playable.

The polygon list code uses evil pointer arithmetics that could really use a rewrite using a chained list class.

I also fixed a few bugs in the data loading procedures resulting in some glitches disappearing from the intro with both 32 and 64 bits builds. The fixed glitches are : - The small boat and the birds in the sailboat screen. - The head of the butler in the following scene.

Ticket imported from: #2054467. Ticket imported from: patches/1016.

Attachments (2)

cruise_64bits.patch (14.3 KB ) - added by bgK 12 years ago.
64bits fixes
cruise_v2.patch (21.6 KB ) - added by bgK 12 years ago.

Download all attachments as: .zip

Change History (13)

by bgK, 12 years ago

Attachment: cruise_64bits.patch added

64bits fixes

comment:1 by sev-, 12 years ago

Yes, nobody worked with Cruise un order to make it 64-bit, endian-, or alignment-safe. We really need to perform this work.

There is an easier and more portable way of loading those structures, so if possible, I encourage you to do that. You may see it well in SAGA engine. It is use of MemoryReadStream. For instance, first encounter of structure loading in overlay.cpp in your patch would look like this:

MemoryReadStream readS(scriptPtr, ovlData->numMsgRelHeader * 34); // better to put 34 to a constant

for (i = 0; i < ovlData->numMsgRelHeader; i++) { ovlData->arrayMsgRelHeader[i].type = readS.readUint16LE(); ovlData->arrayMsgRelHeader[i].id = readS.readUint16LE(); ..... ovlData->arrayMsgRelHeader[i].dialog = readS.readUint16LE(); }

There are functions for reading big endian data, reading bytes, 32 bit words, both signed and unsigned.

Also this patch in ctp.cpp:

- a2 += 4+sizeof(int16*); // skip header + a2 += sizeof(int16*) + 6; // skip header

Looks suspicious to me. Aren't you breaking it 32bit machines?

For chained lists we have class sitting in common/list.h

comment:2 by sev-, 12 years ago

Owner: set to sev-

comment:3 by bgK, 12 years ago

>Also this patch in ctp.cpp: >- a2 += 4+sizeof(int16*); // skip header >+ a2 += sizeof(int16*) + 6; // skip header >Looks suspicious to me. Aren't you breaking it 32bit machines?

I'm not so sure about it too. This function adds a record to a chained list. The record is defined as follows : ptr : next record u16 u16 u16 u16 u16 u16 array of u16 pairs

The line you quoted is used to skip the header of the record in order to write the array. It seems correct to skip a pointer and 6 int16. Skipping only 4 int16 would make the header overwrite the first element of the array. Maybe it is the expected behaviour, but the change doesn't seem to break anything.

Maybe I'll look into the MemoryReadStream thing when I get some free time again.

comment:4 by bgK, 12 years ago

A more correct code would probably be : a2 += sizeof(int16*) / sizeof(int16) + 6; It gives the same value for the 32 bits build and doesn't mix bytes and words.

by bgK, 12 years ago

Attachment: cruise_v2.patch added

comment:5 by bgK, 12 years ago

Attached is an improved version of my previous patch using a stream so that the changes are endian safe. A lot more work is required to make the whole game endian safe though. File Added: cruise_v2.patch

comment:6 by fingolfin, 12 years ago

What is the status of this item?

comment:7 by bgK, 12 years ago

Pending for a review, I guess.

comment:8 by fingolfin, 12 years ago

Indeed. My hope was the sev would tell us what he thinks about this patch ... ;)

comment:9 by sev-, 12 years ago

Somehow I missed the update. Thank you very much. The patch committed.

comment:10 by sev-, 12 years ago

Status: newclosed

comment:11 by digitall, 2 years ago

Component: Engine: Cruise
Note: See TracTickets for help on using tickets.