The source project of this merge request has been removed.

Summary

This merge request fixes a common desync issue related to loops, wherein players entering a loop on a dedicated server would cause a desync.

The testing environment is:

A linux dedicated server compiled with the ninja-release preset (commonly used when hosting a dedicated server).
- Specifically hosted using WSL through an Ubuntu 22.04 installation with the recommended libraries to build the game.
A windows client compiled with the ninja-x86_mingw_static_vcpkg-release preset (commonly used when playing the game).

Bug

Some loops, when taken by players on a dedicated server hosted using Linux, would cause a desync like so:

Test Track:

testtrackdesync2

Stardust Speedway Good Future (from lizardpack):

speedwaygooddesync

While some loops, like Test Run's, would never cause a desync, even in the testing environment described.

Explanation

There's some important details about the desync:

It happens on a linux dedicated server, but not a windows dedicated server, implying a platform-specific issue.
Sometimes it's not consistent, a loop taken under the testing environment may just work without a desync, implying some sort of undefined behaviour.

I decided to print some values in P_SpawnItemRow, in the isloopend block to see where the issue may lie. I noticed that the second end loop was giving values with different signedness, and because of that, it was giving completely different angle values which, in loops.cpp:set_shiftxy(), these would get used and give different shift.x and shift.y values for the looping process for the player, which will then later give a desync during gameplay (left is Linux, right is Windows):

I went a little earlier into the stack, and I printed the mapthing IDs of the loop end points in P_SpawnItemLine - this is where I noticed that, for some reason, the map things were just ordered differently depending on the OS (left is Linux, right is Windows).

Finally, in p_setup.cpp, my eyes laid to the qsort call in in P_SpawnMapThings, and more specifically cmp_loopends, which is the function that dictates how loop ends will be sorted. The comparison function biases towards the things' tags, and then the things' first parameter (in HVR, the "Gate Side"). Generally, to mark a "gate", you need two things, and those things will usually have the same tag and the same gate side parameter. Thus, if two things, with the same tag and gate side, are compared by cmp_loopends, they will be considered equal.

This is where the problem lies. If we look at the documentation in qsort, it says: "If comp [the function used for the comparison] indicates two elements as equivalent, their order in the resulting sorted array is unspecified."

As it turns out, Linux and Windows handles equality differently. Linux takes a sensible approach and sorts things in ascending order if both elements are equal. Windows takes a shot of vodka and throws two darts to determine which one is "higher" than the other.

In Stardust Speedway Good Future (from lizardpack), which has a lot more loops, makes the problem more apparent, specially how Windows will sort both mapthings either in descending or ascending order with no rhyme or reason:

As noted before, when the order of these paired mapthings is unstable, it will sometimes give completely different results when it calculates the difference between the positions of the mapthings and the angle obtained from this difference. It should be noted that this issue would not happen if both of the mapthings were flush in an axis from one another - this is why this desync doesn't occur in Test Run, for example, since those are perfectly flush in an axis, but occurs in Test Track, whose loop end points are offset by a minuscule yet significant amount for a desync to happen.

Nevertheless, the fix is, when cmp_loopends would result in zero, it will instead compare the first mapthing's ID with the second's, resulting in an ascending order consistent with how Linux does it.

When done this way, it results in a consistent sort between both platforms (Test Level and Stardust Speedway Good Future respectively):