Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

Disabled player never get un-controlled #2812

Closed sgkoishi closed 1 year ago

sgkoishi commented 1 year ago

Reproduction steps (if applicable)?

  1. SSC false, RequireLogin true
  2. Join the server.

Any stack traces or error messages (if known)?

https://github.com/Pryaxis/TShock/blob/a00d5ed2a94c26a8a47ec66421b4a6f706b02b77/TShockAPI/TShock.cs#L1193-L1204 The player will be Disabled because not logged in. https://github.com/Pryaxis/TShock/blob/a00d5ed2a94c26a8a47ec66421b4a6f706b02b77/TShockAPI/Commands.cs#L808-L812 They can not log in because they were disabled.

The SSC server will just have the players Disabled once and they can wait till expires.

drunderscore commented 1 year ago

Yup, absolutely, thanks for bringing this to my/our attention.

I had recalled this feature being in TShock, but couldn't find a corresponding config entry for it, but obviously this is it.

We can either:

I will end up PR-ing the first option ASAP, as this is obviously an unacceptable behavior, is an acceptable behavioral change (imo), and we shouldn't be superficially waiting on possible implementation of changes by Re-Logic.

drunderscore commented 1 year ago

So unfortunately my implementation of option 1 fell short...

The Player.CCed property represents the player being either frozen, webbed, or stoned. These states are gathered from the fields Player.frozen, Player.webbed, and Player.stoned respectively.

However, these fields are not always representative of the player's active buffs, depending on when the buffs were applied.

These fields are reset to false in Player.ResetEffects, which is called for every Player.Update. These fields are then updated again in Player.UpdateBuffs, based on the player's active buffs, which is called for every Player.Update, not too long after Player.ResetEffects.

This essentially means that the player on the client would need to tick at least once for the buff removal to perform what we want it to. This means that I won't be able to implement what I described earlier at all.

For now, I will simply relax the restriction for CC'd players during login to only occur on servers with SSC enabled, as that is only where it can cause issues, and additionally, it would allow the RequireLogin config option to once again continue functioning usefully.