gcc/Linux fixes - #71
Conversation
This crashes on Linux as pKeyValuesData is NULL when deleteThis() is invoked. This likely is caused by another issue, but fixing this here seems good regardless.
This is important for case-sensitive filesystems/operating systems (i.e. Linux).
Print them using "%s" instead, so their contents are not accidentally interpreted as format specifiers.
z33ky
left a comment
z33ky
left a comment
There was a problem hiding this comment.
Left a couple of notes about changes that may look funny.
Feel free to ask if there are further changes that look questionable or something is still unclear.
|
Thanks, I did a quick glance through the changes and everything looks pretty reasonable. I appreciate you took the time to clean up some other bits in there too from the looks of it. I'll try to test this as soon as I can and will look to get it merged soon 👍 |
There was a problem hiding this comment.
A note to the changes here: The ## operator is for creating a singular token. In this instance, for example, pasting Create_ with className with ( will never create a valid singular token. Here two tokens are to be created: Create_##className (.
gcc is more strict than MSVC here in that it rejects the operation if pasting fails.
There was a problem hiding this comment.
Passing a temporary via reference is only allowed for const&. gcc disallows doing that if the reference is not specified as const.
In most cases I simply changed the function parameters to include const, however since InputTurn{On,Off}() is used in multiple places I was a bit lazy to check if that's a reasonable change and opted to put the temporary in a variable instead.
There was a problem hiding this comment.
CPropVehicleDriveable is not defined for this project, hence there is no type-info available to do the dynamic_cast with. I don't know what MSVC compiles this to, with gcc this results in a linker error due to missing symbol.
My best guess to implement this using CBaseTFVehicle is just below.
There was a problem hiding this comment.
There is no macro specific to this project. I assume that other code has been changed with the assumption, that no other variant(/game) is compiled from this fork.
To cleanly separate mod-specific changes it may be a good idea to add such a macro though. TF_DLL is already taken, MOD_INVASION perhaps?
If you don't think this approach I can just #if 0 [old code] #else [new code] #endif, or flat out remove the irrelevant code.
There was a problem hiding this comment.
Personally I've not been concerned about seperating the mod-specific changes as I'm not expecting Valve to ever update the SDK at this stage - I'd prefer just to remove the irrelevant code.
There was a problem hiding this comment.
It may be interesting to merge other forks, like Mapbase (if it gets an MP port, mapbase-source#77).
There was a problem hiding this comment.
Leaving w uninitialized is problematic when the VectorAligned is copied, since uninitialized data would be read, which means undefined behavior occurs.
Perhaps w could just be removed, it seems totally unused. Maybe it's required for ALIGN16 to work right? It seemed safest to just initialize it with some dummy value.
There was a problem hiding this comment.
The manual instantiation is unfortunate, but according to a gcc warning the __vector-specifier would get lost in the templated-code, meaning that (slow) unaligned access to the floats was used. By manually expanding CStridedPtr the compiler can emit the proper vectorized access instructions.
I don't know if MSVC actually just does carry over the specifiers or like the old gcc silently throws it away.
6cf5449 to
eddfeaa
Compare
|
I have a minor issue with font loading. For some reason the engine is unable to find the fonts ("Couldn't find custom font file") if they're three directories deep (e.g. "resource/fonts/Squada_One/"). The class-selection menu is bugged for me; clicking the buttons does not work. Pressing "enter" will close it, without selecting a class. Afterwards I managed to change classes using the "changeclass" console command. I'm wondering, is this also broken on Windows? I'll probably take a look at what's going on there. |
If you do, can you rename the OFL in the same directory to use the same name as the font? Basically
Both the team selection and class menus should work, so I'm not sure what's going on there. |
Yeah sure.
I figured out it's some undefined behavior in the VGUI code - also present in the upstream Source SDK. That bug just became apparent by using a newer compiler, probably some optimization which killed the intent. |
We don't use CPropVehicleDriveable. Instead we've got CBaseTFVehicle, so check for that.
The HL2(MP) assets weren't being loaded correctly on Linux. I copied the settings from the original Source SDK 2013 file, which fixed it.
This works around an engine bug that seems to exist for Linux, where fonts are not found if three directories deep: Couldn't find custom font file 'resource/fonts/Squada_One/SquadaOne-Regular.ttf'
The button mask is created by shifting a bit according to the MouseCode, which is just a renamed ButtonCode_t. Mouse buttons start at 107, which is way out of range for our ints. To fix this, introduce MouseButtonBit(), which checks if a button code corresponds to a mouse button at all and returns a usable bitmask for the corresponding mouse button code. This is then used for the button mask.
eddfeaa to
c860df4
Compare
| if(state) | ||
| { | ||
| //set bit to 1 | ||
| _mouseClickMask|=1<<((int)(code+1)); |
There was a problem hiding this comment.
I don't know why they did that +1 here. In MouseButtonBit() I use bit #0, which they may have wanted to avoid for some reason.
There was a problem hiding this comment.
Who knows, VGUI is a horrible poorly documented mess. If that resolves it I'm happy but should probably double check for regressions elsewhere before merge.
2 participants
Hi,
I got this thing to compile and run on Linux. Mostly it's just fixing file paths, since in Linux that's usually case-sensitive.
There were a couple of compiler errors, mostly easy to fix, and I squashed a few warnings as well.