Possible unneeded packing of structs

[th-otto]

There are some structures like

Vanilla-Conquer/common/combuf.h

Line 60 in 77e2da0

typedef struct BITFIELD_STRUCT

that are declared with pack(1) and also attribute((ms_struct)) However, since they also contain pointers, they are certainly not used anywhere where files and/or network data is accessed. Declaring it in the current way causes inefficient code to be used on architectures that don't allow unaligned accesses, and also slows down accesses to the member on architectures that allow it.

[OmniBlade]

Its possible they were swept up in the work to get cross platform LAN play working and don't need to be packed or the bitfield done a particular way. If you want to test network play between a client that has had these statements removed vs one that hasn't and verify they are still able to connect and play then they are probably safe to remove.

[th-otto]

I have a bit difficulty in testing this, since you apparently cannot run both the client and server on the same machine.

BTW, i also found that in tiberian dawn, a lot of network related types are duplicate in defines.h and session.h. But unlike redalert, session.h does not seem to be used at all.

[ChthonVII]

Just FYI, pretty much none of the struct packing matches the original game. The old Watcom compiler always padded bitfields to the nearest byte, while MSVC (and I think other modern compilers too) pads them to the length of the longest type of any member of the bitfield. So something like unsigned int IsActive : 1; causes the bitfield to be padded to a multiple of 4 bytes in MSVC, but it would have been padded to the nearest byte in Watcom. (unsigned int is super common in bitfield declarations all over the C&C codebase.) Consequently, (1) there's not as much space saving as you'd think, (2) there's often better alignment than you'd think, and (3) the padding Petroglyph added to various structs is often the wrong amount to align to word boundaries.