Multiplayer big endian fix#509
Conversation
|
Nice! What was the big endian machine that you had connected with the windows box for multiplayer? Would love to see a couple of pics of it working :) |
It was MorphOS running on an iMac G5 and Mac Mini G4. We didn't snap a whole lot of interesting pictures, it was mostly looking at logs, but I'll try to find something that's presentable. |
|
"Ron host" was the windows machine, but we tried all kinds of combinations. Variable length messages were a pain in the rear end. |
| } | ||
|
|
||
| #if BR_ENDIAN_BIG | ||
| static void NetSwapVector3(br_vector3* pVector) { |
There was a problem hiding this comment.
I think to make it super clear that this code is added and not part of the original retail code, could we move these functions into a separate file (network_bigendian.c ?) that we could mark as added?
Then we can guard the calls into those functions from network.c by #ifdef DETHRACE_FIX_BUGS
There was a problem hiding this comment.
I can move them to a separate file, but I'm not sure if DETHRACE_FIX_BUGS would make sense in this case. The other byteswaps are not guarded by it and it's technically not a bugfix, so if possible I'd prefer to keep BR_ENDIAN_BIG to be consistent.
There was a problem hiding this comment.
Would this work too Jeff?
network_endian.h
#ifndef NETWORK_ENDIAN_H
#define NETWORK_ENDIAN_H
#ifdef BR_ENDIAN_BIG
#include "dr_types.h"
static void NetSwapMatrix34(br_matrix34* pMatrix) {
for (int row = 0; row < 4; row++) {
for (int col = 0; col < 3; col++) {
pMatrix->m[row][col] = BrSwapFloat(pMatrix->m[row][col]);
}
}
}
#else
#define NetSwapMatrix34(MATRIX)
#endif // BR_ENDIAN_BIG
#endif // NETWORK_ENDIAN_HAnd use NetSwapMatrix34(matrix) unconditionally.
Then these functions/macro's don't show up on little endian platforms.
There was a problem hiding this comment.
NetSwapMatrix34 is a static function inside an if BR_ENDIAN_BIG block, so it won't show up in little-endian executables at all.
There was a problem hiding this comment.
Indeed. The key difference is that the network.c source will not add extra "unrelated" functions, close to how it was done in 1997. Using a nop NetSwapMatrix34 macro also avoids adding lots of #ifdef BR_ENDIAN_BIG/#endif macro conditionals.
There was a problem hiding this comment.
I'm not sure I understand, maybe you mean NetSwapMessage, NetSwapContents and NetSwapMessageAndContents?
There was a problem hiding this comment.
NetSwapMessageAndContents
Yes! The name of the function/macro should be identical in both branches! Sorry about that :) (I edited the suggestion)
There was a problem hiding this comment.
Ah okay, I get it now. I can put the whole thing into a header with static functions. If there are no macro guards, then maybe a comment like // dethrace added would be nice for swap functions.
There was a problem hiding this comment.
yes, that would work. I don't mind there being extra functions in the binary, but the original functions must be compilable to the original assembly for binary matching (BR_ENDIAN_BIG will not be defined during binary matching). What @madebr suggested would fit that constraint 👍
There was a problem hiding this comment.
I moved the NetSwap* functions into a separate header file, but I couldn't bring myself to remove the BR_ENDIAN_BIG macro guards yet. There are only 8 of them in a 2200+ line file, and removing them makes the code less searchable for BE fixes. It also feels like we are obfuscating why these functions were added. Please check network.c now, and let me know if it'd be OK for you to leave it like this.
| #include "structur.h" | ||
| #include "utility.h" | ||
| #include "world.h" | ||
| #if BR_ENDIAN_BIG |
There was a problem hiding this comment.
BR_ENDIAN_BIG is always defined as 0 or 1 by CMake or by BRender's compiler.h. Actually it should be #if in network_endian.h too, I'll fix this.
3 participants
Bit endian clients can now play with little endian hosts and vice versa. This was tested with the Dethrace 0.9.0 Windows release.