#8911 closed patch
CRUISE: 64bits fixes
| Reported by: | bgK | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Cruise |
| Version: | 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)
Change History (13)
by , 16 years ago
| Attachment: | cruise_64bits.patch added |
|---|
comment:1 by , 16 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 , 16 years ago
| Owner: | set to |
|---|
comment:3 by , 16 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 , 16 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 , 16 years ago
| Attachment: | cruise_v2.patch added |
|---|
comment:5 by , 16 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:8 by , 16 years ago
Indeed. My hope was the sev would tell us what he thinks about this patch ... ;)
comment:10 by , 16 years ago
| Status: | new → closed |
|---|
comment:11 by , 6 years ago
| Component: | → Engine: Cruise |
|---|
64bits fixes