TTT-2 / TTT2

Trouble in Terrorist Town 2 for Garry's Mod (gmod)
https://steamcommunity.com/sharedfiles/filedetails/?id=1357204556
179 stars 77 forks source link

Investigate ply:Alive() #1450

Open TimGoll opened 8 months ago

TimGoll commented 8 months ago

In our code base we sometimes check if a player is alive: https://github.com/search?q=repo%3ATTT-2%2FTTT2%20%3AAlive()&type=code

However I'm not sure there is any scenario where a player isn't alive. Spectators are counted as alive as well, therefore we differentiate with IsTerror and IsSpec.

This should be investigated, maybe we can get rid of all Alive checks. We also have quite a few places where we explicitly check the team for TEAM_TERROR or TEAM_SPEC. I think we should replace those with IsTerror and IsSpec

TimGoll commented 8 months ago

Looking at the wiki it looks like a player can indeed be dead: https://wiki.facepunch.com/gmod/Player:Alive

However this might be overwritten in TTT to keep spectators alive as well?

Histalek commented 8 months ago

Would a disconnected player still be "alive"? maybe this check is needed in some places?

TimGoll commented 8 months ago

It seems like we do kill players in TTT2, but we respawn them in the same tick again

Example: player changes to being a forced spec: https://github.com/TTT-2/TTT2/blob/ded4f19fa53f737c223c62f23f05bd1fc971d933/gamemodes/terrortown/gamemode/server/sv_networking.lua#L492-L500

TimGoll commented 8 months ago

Would a disconnected player still be "alive"? maybe this check is needed in some places?

I don't think so, because a disconnected player is a "NULL ENT"

TimGoll commented 8 months ago

Stuff worth looking at:

TimGoll commented 8 months ago

So I get the hunch that there are scenarios where a player is not alive, because the wiki says Alive will return false in DoPlayerDeath. But it seems like we respawn the player as a spec immediately and only set the team? I'm a bit confused because I'm unable to locate this in the code base

TimGoll commented 8 months ago

This is all I'm able to find, here is the team updated from TERROR to SPEC. But a Spawn is never called, so a player should still be counted dead, so Alive should return false? I'm really confused and I have to test this in-game at some point:

https://github.com/TTT-2/TTT2/blob/ded4f19fa53f737c223c62f23f05bd1fc971d933/gamemodes/terrortown/gamemode/server/sv_player.lua#L829-L832

mexikoedi commented 8 months ago

It could be worth looking into the IsActive() check as well. Alive players in an active round so it cannot be a spectator or a dead player. Could be used in some places to simplify the code instead of using IsTerror() and IsSpec() checks. See here: https://github.com/Facepunch/garrysmod/blob/61cb86af65daa454e0d879be78bc9b40fe956ce2/garrysmod/gamemodes/terrortown/gamemode/player_ext_shd.lua#L23

TimGoll commented 8 months ago

But IsActive only checks IsTerror and if it is an active round. So I don't think there are many places in our code base where this could be simplified

EntranceJew commented 8 months ago

This is all I'm able to find, here is the team updated from TERROR to SPEC. But a Spawn is never called, so a player should still be counted dead, so Alive should return false? I'm really confused and I have to test this in-game at some point:

https://github.com/TTT-2/TTT2/blob/ded4f19fa53f737c223c62f23f05bd1fc971d933/gamemodes/terrortown/gamemode/server/sv_player.lua#L829-L832

This is accurate, all the shuffling happens immediately after death, and in a confusingly named "respawnforround" function, in the snippet you cited: That :Freeze(false) should be skipped if the player is a bot, as it enables valve-bots to wander out of the map boundaries -- to be consistent with the other places the player is being frozen