TheAssemblyArmada / Vanilla-Conquer

Vanilla Conquer provides clean, cross-platform builds of the C&C Remastered Collection and the standalone legacy games.
Other
358 stars 54 forks source link

attribute(ms_struct) is only valid for x86 #980

Open th-otto opened 8 months ago

th-otto commented 8 months ago

That damn clang-format is driving me mad ;)

Unfortunately i cannot check that before pushing the commit, because clang format version 16 installed here produces slightly different results. Is there some way to adjust the settings such that they behave the same?

A diff after running clang-format is attached

format.diffs.txt

OmniBlade commented 8 months ago

Yeah, clang-format not being predictable between versions is a great source of annoyance and makes using shared clang-format configurations much more difficult than it should be.

Regarding the changes, if you can make the padding consistent across all platforms without the attribute, the code can be simplified by removing the conditional stuff and remove the BITFIELD_STRUCT macro entirely, though this will then require some testing to ensure cross platform networking still works.

th-otto commented 8 months ago

That might be possible. However for the bitfields, you'll still need conditionals for big-endian systems. Of course that must be done carefully, but i've already added new tests to check the current layout.

BTW, if i'm not wrong, then only red alert currently takes care of swapping the packets that go over the network. For Tiberian Dawn, that seems to be still missing.

I also want to check the EventClass type. If i'm not wrong, that is not used anywhere in the network code. It only affects MAX_IPX_PACKET_SIZE for whatever reason.

giulianobelinassi commented 6 months ago

@OmniBlade @hifi @tomsons26 what do you think?

OmniBlade commented 5 months ago

I'm happy to replace the attribute(ms_struct) with explicit padding that makes the bitfields predictable everywhere. No point using it to smooth platform differences if it doesn't work everywhere.