Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plmenu: Fix another alive spectator bug #175

Closed
wants to merge 2 commits into from
Closed

plmenu: Fix another alive spectator bug #175

wants to merge 2 commits into from

Conversation

Freeman-AM
Copy link
Contributor

Switch a newly connected user to spectator who selected his team but not his class can create an alive spectator if done during the first seconds of the round.

Solution : block the team transfer for any people stuck in this menu.
Well it's the worst case and probably the only possible one to completely fix this issue.

Switch a newly connected user to spectator who selected his team but not his class can create an alive spectator if done during the first seconds of the round.
@Nextra
Copy link
Contributor

Nextra commented Jan 7, 2015

Executing a "joinclass" engclient_cmd should be enough to properly resolve this situation, allowing a team switch without bugs.

@Freeman-AM
Copy link
Contributor Author

Well, it was the case before. But i forgot something.

@Nextra
Copy link
Contributor

Nextra commented Jan 7, 2015

So what happens exactly? Which mods does this problem occur on? For CS the joinclass method really should be enough. It may have to move to a different spot to work properly.

@Freeman-AM
Copy link
Contributor Author

It's the default plmenu who create this problem. At round start, normal players who join the party can spawn after the start of the round.This method doesn't work at all in fact, because if it's at round start, the spectator can still spawn. I don't think this is normal to let a spectator spawn. And the only reason why joinclass was here is to fix that. but if it's a failure, why keep it ? (if you don't use fakemeta, it's necessary to keep it to avoid to crash a player and probably a server)

@Arkshine Arkshine added the bug label Jan 10, 2015
@Arkshine Arkshine self-assigned this Jan 11, 2015
@Arkshine
Copy link
Member

If I understand, with original code: a player chooses team, but not class. Round starts. An admin switches player to spectator. Player respawns because of joinclass 6 and that player is allowed to spawn at this point, then plugin tries to kill player, and does jointeam 6 to transfer to spectator. At this point you're saying that plugin failed to kill the spawned player, but jointeam worked, which result a spawned spectator, right?

@Nextra
Copy link
Contributor

Nextra commented Jan 14, 2015

Player joins the server and selects a team (but not a model). Executing the joinclass command will set the model correctly for the team he is on. Crucially it also sets m_iJoiningState = GETINTOGAME which will then make the player spawn on the next server frame as CBasePlayer::JoiningThink does not check the team the player is on. As this happens on the next frame any is_user_alive() safeguards immediately within this function will not work properly and we have an alive spectator shortly after.

Here are some ideas for fixing the issue:

  1. Don't execute the joinclass command when moving players to spectator. I don't know if this has any side effects so this would require further testing. Probably have to cancel the open choose appearance menu.
  2. Set m_iJoiningState = JOINED as we already work with Fakemeta now, thus preventing the player spawn on the next frame.
  3. Work with the spectate command. Although that really only does jointeam 6 anyway, which we already have in this function for the non-cs codepath. I don't know why this was split up originally, but for switching people to spectator using the alternate codepath might actually be the better solution (may also allow us to remove the awkward Fakemeta code from this plugin).

Of course this is only considering CS, and I don't know if there are possible issues with all of this in other mods, too.

@Arkshine
Copy link
Member

Solution with m_iJoiningState is probably the cleanest as when you choose spectator from team menu, this offset is set to JOINED. Setting with m_bTeamChanged is probably a good place for that.

Disallowing could have been an okay solution if it was available for this short period of seconds where a player can respawn anytime. But all the time doesn't seem like a good idea as this can be forced without issues and this will probably annoy admins.

Removing fakemeta code to enforce joinclass 6 is probably okay.

So to fix patch, I would remove the new block of code above, and adding the new offset. This should be okay this way.

@Freeman-AM
Copy link
Contributor Author

http://pastebin.com/j5vNXCDH
Solution with m_iJoiningState (121 for amx) don't work as expected.
We can't control the camera (act like an unassigned team camera) and there is a skin bug as you can see.
I had the same problem when i blocked the spawn with Ham.
test
de_dust20002
de_dust20003

@Arkshine
Copy link
Member

I did not say to remove joinclass 6, it's still needed to unstuck player. The offset is there to avoid player be respawned the next frame.
I was talking obviously about the block which disallow transfer if player is stucked in appearance menu.

@Freeman-AM
Copy link
Contributor Author

http://pastebin.com/nRc31SqG
I obviously know, but even with joinclass, as in this code, i have the same problem, that why i removed it, for testing.

@Arkshine
Copy link
Member

It works for me. I don't know if I test properly, but I do the following:

  • I choose team but not appearance
  • I'm waiting for round start
  • I send a command to execute: joinclass 6 + m_iJoiningState to 0 + jointeam 6.

And I don't respawn and well in spectator team.

@Freeman-AM
Copy link
Contributor Author

Okay, so the problem is the plugin and a wrong if statement calling a block (cs_set_user_team block) rather the correct one.

This part of the code seems to need more rewriting.

@Arkshine
Copy link
Member

Maybe you don't have fakemeta enabled ? :P

@Freeman-AM
Copy link
Contributor Author

I have fakemeta enabled, it's just that at the plugin current state, no jointeam 6 is called after or before m_iJoiningState JOINED and joinclass 6.

It seems needed.

@Arkshine
Copy link
Member

I would say that because of g_CSPlayerCanSwitchFromSpec[player] being basically always true whatever you select a valid team. It's probably okay to pass in this check, but at this point, it needs to add a check that if we select spectator then we set m_iJoiningState to 0 so we don't respawn. Not sure if we should just use cs_set_user_team instead of using jointeam. Probably cleaner to work with the game.

Code is quite messy :/
This would need really to be rewritten properly.

@Freeman-AM
Copy link
Contributor Author

I close this one for the moment.

@Freeman-AM Freeman-AM closed this Jan 24, 2015
@Freeman-AM Freeman-AM deleted the alive_spectator_bug branch January 24, 2015 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants