64bit conversion

[flukejones]

Noticed the requirement is for 32bit SDL builds. Given that MacOS will be depreciating the ability to run 32bit binaries in the very near future, what are the plans for 64bit support?

[Interrupt]

64 bit support would be great, but it's not a priority at the moment. Strongly typing all of the structs that are serialized might be the bulk of this work. Want to give it a shot?

[rfht]

I would also be interested in 64bit conversion because that would allow porting it to OpenBSD where we don't have 32bit compat on 64bit arches to begin with. @Luke-Nukem if you want to collaborate on this, let me know.

[speachy]

A blanket conversion of C types to C99 types would be a good start.

(u)char->(u)int8_t, (u)short->uint16_t, (u)int->uint32_t should be harmless, but (u)long is where the real fun lies -- it's used as a (u)int32_t in some places, but as a pointer in others.

[rfht]

thanks, that should help with getting started :)

[inguin]

The grs_bitmap structure is a tricky case: It contains a pointer (bits) to the actual image data. In the resource files the value of the pointer is always 0, and it is fixed up after loading to point directly behind the structure. On 64-bit systems the reserved space is too small to hold a pointer.

For static images the pointer could easily be replaced with a zero-length array at the end of the structure. This would automatically have the correct address, without need for fixing up. Unfortunately there are many places where images are allocated dynamically, so those would need to be adapted more heavily.

[winterheart]

@speachy There's plenty 64-bit related traps hidden in code, simple replacement is not enough. At least RES and FIX (PR #29 pending) are 64bit compatible, but at this time forced 32bit environment is best choice.

[icosahedral-dragon]

So I had a look at getting a build going. There are a few categories of 64-bit gotchas in the code. (Well, it was 1994. I doubt the possibility ever occurred to any of the developers that anybody would be trying to build System Shock on 64-bit machines 25 years later.)

The code is generally sloppy about types in a few places. Probably benign, but I'm building with -Werror to catch potential problems. Easy enough to fix.

Lots of casts between void* and int or other integer types. These tend to be callbacks taking a generic parameter. I've changed a bunch of these to intptr_t since that's what it's for. Also there are architectures out there that will barf on an invalid pointer value even if it's never dereferenced.

There are a couple of functions in AFILE that were apparently mechanically decompiled from a file whose source code wasn't available for some reason. These assume that any value that can go into a register can be freely converted to and from 'int'. There was nothing for it here but to rewrite those functions in something a bit more closely resembling C. I haven't actually tested these and I'm not even sure they're used in the current version, but it gets the code to compile. (And that's all that really matters, right?)

At this point the code compiles and we're looking into crashes.

Some header files invoke #pragma pack and don't restore it afterwards. This causes problems if a library header (specifically SDL) is subsequently included that expects a different packing. The code will crash because SDL will create a structure at one packing and the game will attempt to use it at a different packing. Always restoring the packing at the end of any header that uses a #pragma pack fixes that crash.

The game assumes that the in-memory layout of various structures matches the on-disc resource layout. Regardless of packing this will never work for a structure with pointers in it, as the aforementioned grs_bitmap (FrameDesc on disc) has. This is where the 64-bit port stops being a mechanical translation exercise and starts requiring some actual programming chops. But I have a plan. My idea is to add a pointer to the ResDesc structure to hold the decoded data, freed when the raw data is freed. You can then deserialise a resource by passing a struct describing the resource layout. It should be a relatively minor change to the resource manager that will decouple the in-memory from the on-disc layout of a given structure. I'm hopeful that the game will actually run then.

[icosahedral-dragon]

I'm not quite sure I got the update to the anim library quite right ...

[winterheart]

Welcome to bizzare world of C porting platform adventures.

[icosahedral-dragon]

It's more the bizarre world of C's infamously baroque operator precedence rules, it turns out. A couple of strategic brackets got the intro video to play almost correctly; there are still a few stray pixels. Quite a few more substitutions of int32s for longs and some correction of memory size calculations that assume a given size for datatypes and the game begins to be playable. Now, why doesn't it realise that cyberspace maps are actually cyberspace?

[winterheart]

Just reviewed your changes, few advises.

• Convert int types to (u)intXX_t from stdint.h - this allows to get rid some compatibility defines.
• Make your changes as series of short pull requests divided by functionality (2D, 3D etc), it makes easy for regression testing.

[icosahedral-dragon]

Sure, I'll try and bash it into a coherent set of patches once I've got stuff working. I haven't tested that it still works in 32bit ... ironically enough, I've had more success getting the 64-bit config to run than the 32-bit, something seems not to be configured in my system. Ideally we'd have a set of smallish, self-contained commits that work towards 64-bit functionality without breaking 32-bit.

[Interrupt]

It is a bit ironic that building 32 bit apps is so hard lately, this project started out on OSX but since then they've mostly degraded support for 32 bit apps to the point where building a 32bit OpenGL app is impossible.

[rfht]

I just would like to report that 64bit works on OpenBSD amd64 with a merged version of master from Interrupt/systemshock and 64bit-proof-of-concept branch from icosahedral-dragon/systemshock. Intro video plays nicely and I can walk around in the 3D environment of the game and interact with things. Haven't explored past the first room yet.

The 2 things that are off:

• The midi music is kind of chaotic - the timing is off; I can recognize the music, but it's not 100% correct.
• I can't get mouselook to work it seems. I haven't done much exploration, but thought it would work out of the box which it doesn't.

The merged port is currently on my CVS repo only with a ton of patches derived from the 64bit-proof-of-concept branch:

https://thfr.info/cgi-bin/cvsweb/mystuff/games/shockolate/

Thanks for the great work on this!

[rfht]

PS: I uploaded a screen recording of what it looks like on OpenBSD: https://youtu.be/nlVBbvxHZzY

[icosahedral-dragon]

Thanks @rfht for your report. I'll have some more patches to submit to the master in due course, but it'll probably be a little while before we can get the 64-bit port fully merged. The main thing that is yet to be done is the serialisation (saving) code for resources: at the moment resources are converted where necessary when loading but not when saving. So saving and reloading a game, or attempting to reenter a level you've already been to, results in a crash. This means that for example you can go into cyberspace on the medical level and look around and it works fine, but the game crashes when you exit.

Midi music is indeed a bit off. I'm not sure whether it's a 64-bit or a general problem; I'll check with the 32-bit build at some point. Mouselook works fine for me on Ubuntu Linux 16.04 in 32 and 64-bit builds. It may be an OpenBSD-specific problem.

Thanks for the video as well. I meant to upload one myself but have been too lazy so far :)

[rfht]

I think the mouselook issue is more likely something that I messed up when combining master and 64bit-proof-of-concept branches. I haven't seen any issues with mouse controls in OpenBSD ports so far.

Thanks so much for working on this, @icosahedral-dragon !!

[Interrupt]

@icosahedral-dragon breaking the save format to support 64bit is something that we should be fine with if that is an easier path, the save version number seems to be built for cases like this.

[Interrupt]

Update: was able to get the 64bit branch running in OSX, the music seems to be playing back just as well as it does in the 32bit build, things are looking great so far!

There’s a crash on startup that I had to work around and I’ve been trying to isolate, seems like it happens in event.c in uiPoll when processing mouse events. Usually happens when setting the buttons here:

https://github.com/Interrupt/systemshock/blob/9c37dd2f14893dba6fee8ba07785296663598d4f/src/Libraries/UI/Source/event.c#L902

[icosahedral-dragon]

I'd rather keep the save format the same and explicitly serialise the data structures; we'd need to keep backwards compatibility on load anyway, and binary format isn't dependent on code version so much as architecture, compiler options, phase of the moon ...

As for the crash in event.c: the uiEvent structure is one of those generic structures that's padded out explicitly to the size of the largest specific variant that might need to be put in it ... or so it thinks. But the mouse event struct has a ulong in it. This sort of thing would be better handled with a union, but changing the ulong to a uint32 in this line: https://github.com/Interrupt/systemshock/blob/9c37dd2f14893dba6fee8ba07785296663598d4f/src/Libraries/UI/Source/event.h#L91 should fix the crash.

[winterheart]

We can redefine ulong to uint32_t altogether in lg_types.h for awhile.

[Interrupt]

@icosahedral-dragon you should drop by the Shockolate Discord, we've been discussing your work a bit over there!

https://discord.gg/kvXfua8

[winterheart]

Sooo, seems 64-bit effort mostly done. Funny, but seems now we have opposite problem with 32-bit build... Anyway, I think, we can close this issue.