Various gameplay & stability fixes

• Gameplay

• 58823381 - Fixed "magnet landing" - a physics bug where players would "magnet" to a previous position after being hit by a wipeout-inducing item, use a spring panel sector, and then landing.

• e123f4bd - Fixed "0th position" - a bug where its most common side effect was, when a player's roulette finished, if someone else respawned on that same tic, the former player would get significantly better items (the respawning player would have position 0th for a tic, which is technically higher than 1st, thus 1st's position gets compared against 0th's to determine useodds).

• dfc942a6 - Fixed drift charge disappearing in midair, if the player performed a new drift in midair, meaning the player's drift charge would disappear.

• df769a13 2c1d5a72 - Addressed yellow spring panels removing grown players' miniturbos and causing wrongdrift, by making an exception to the minimum speed required to drift if the player is in yellow spring panel state, and has a higher scale than normal. This is because otherwise, for at least one tic, the grown player's speed will always be below the minimum speed, causing undesired effects.

• bf911d02 - Thunder Shield now has COOLDOWNONSTART when rolling for items. Commit 9e6980c8 changed how the Thunder Shield works, inadvertently removing the starting cooldown that existed pre-1.3 (which was part of the bug), but many believed the cooldown component was an intentional mechanic (because Thunder Shield at the start of races is really strong), so this has been added back.

• Stability

• 58823381 - Removed unused painstate value in MT_SINK, which could cause crashes due to map and gameplay elements that could hurt the Sink (namely, Laser FoFs).

• 58823381 - Added MF_NOCLIP in MT_BOOMPARTICLE's flags. This namely fixes Jawz's smoke effect hanging the server and clients if the object moved out-of-bounds, as the combination of flags had it run the scenery thinker and not account for this unless MF_NOCLIP was present.

• 733cf20a - GS_LEVEL gamestate is now set after the level finishes loading. Redid this, see below.

• 4fdbfb1d - Don't handle connections until the level finishes loading. This fixes a myriad of join bugs, such as dedicated servers crashing if the generated save game was invalid, clients crashing for downloading a corrupted or partial save game, listen servers infinitely hanging if someone joined as the server started, and various zombie client issues.

The following is attached:

• A 32-bit Windows build of the game with this branch's fixes, with DEVELOP flag. srb2kart_morefixesjug_2.exe
• A test level and script for testing the magnet landing glitch, MagnetLandingTest.wad. Do map map01, then use the throworb command in front of a spring panel to see it in action. MagnetLandingTest.wad
• A test script for testing the 0th position bug, 0thpositionrepro.lua. Load the script, then load a level, then use the 0thpos command with 2 players to see it in action. 0thpositionrepro.lua

Feedback is much appreciated.

Taking a look at the Grow drift fix, did you consider mobjscaled maps? I noticed it makes an absolute comparison of size vs FRACUNIT ... isn't it entirely possible that a grown player will be smaller than FRACUNIT if they are in a map that has a small mobjscale? Is it also possible to have a player that is always larger than fracunit in certain maps? You should probably check using grow state, not scale.

I did not consider mobjscale. Will fix this soonish. Edit: fixed in 4d8d5d7f

There are custom levels where the player can be bigger without grow state. Shrink-Grow Base comes to mind, even if yellow spring panels are not present there. The check in scale is because the problem is strictly related to the minimum drift speed when scaled up, and not the grow state itself.

(the respawning player would have position 0th for a tic, which is technically higher than 1st, thus 1st's position gets compared against 0th's to determine useodds).

Nice catch on the cause of the problem, but I feel like the fix you've engineered is a bit of a hack, and having a k_position vastly different from the one you deserve for your 3d space coordinate could cause more problems down the line. Ideally this should be handled with a twofold front via a solution for each malfunctioning component.

• Easy and direct fix: Make item roulette code ignore all players with k_position of 0 except for the player who is rolling an item even if they have k_position 0
• Slightly trickier fix: Call the "figure everyone's position out" function (I forget the name) immediately after all PST_REBORN requests for that tic have been processed, as in, literally the earliest possible line in the code, rather than waiting for the next tic to consider this

I'm also not sold on how you've gone about the GS_LEVEL gamestate set location shift. Notably, the preticker is processed inside the context of P_SetupLevel, which means there are two tics where in-level Lua scripts will be called, but functions that require INLEVEL (preprocessor macro for gamestate == GS_LEVEL) will complain out the wazoo and break. It makes on-paper sense that you shouldn't set GS_LEVEL until the level has started, but there's a point in P_SetupLevel where it can no longer return early, and I'd wager that the gamestate set should occur at that cusp instead.

(I have no other issues with this MR pending your addressing of mobjscale for growdrift, but you should be aware you linked the wrong commit for the COOLDOWNSTART set.)

added 1 commit

• 4d8d5d7f - More robust check against yellow springs on grown players.

Compare with previous version

changed the description

Thank you for the feedback, @toaster.

On a future commit I will do your described former fix for the 0th position bug, to avoid position discrepancies (at the time, I just wanted to avoid massively overpowered items on 1st). Edit: fixed in 63f87b4c

On the GS_LEVEL fix, do you think setting GS_LEVEL inside P_SetupLevel would be for the best? Ideally so long as a proper save gets generated, after all of the level's been loaded, the bugs I've described should be avoided. I've set it after P_SetupLevel's execution to more or less keep the code the same - I know people hate it when stuff gets moved around like that so, at the time I abstained against it, but I'd like to hear your thoughts.

Edit: the erroneous commit being linked in the thunder shield log is fixed as well.

added 1 commit

• 63f87b4c - Use toaster's approach to fix 0th position

Compare with previous version

Well, simply swapping that around outside the function introduces a different bug, so a proper fix requires more thorough code modification.

I figured, thanks. I'll work into implementing a different way of tackling the issue.

added 1 commit

• 4fdbfb1d - Abstain from handling connections instead if the level is loading

Compare with previous version

changed the description

I think that should address everything code-wise. Instead of touching game-related code I've decided to have the server not handle connections until the level is done loading, this way the same kind of bugs can be prevented. Let me know if there's anything else that should be tweaked.

changed the description

Updated the Windows build with the new changes.

merged

mentioned in commit a0f0b473

mentioned in merge request !281 (merged)