Revert "replace weapon_switch with ClientCommand (#570)"

[Rainyan]

Description

When spawning in a round, the primary weapon you choose does not automatically activate like it should. This breaks both weapon selections for players, but seems to affect bots too. The player is left in the pistol animation without any weapon selected, and pressing +attack does nothing in this state.

This is a regression from d209d44; the parent commit 51a1ef4 does not observe this behaviour.

I propose we revert #570 (commit d209d44), because it was intended to fix #569 but doesn't, so it's only introducing new bugs currently.

To reproduce the problem

• Join a server
• Join a team
• Choose a class
• Choose a loadout

Expected behaviour

• Your chosen loadout is activated as you spawn in.
• Bots will also spawn with their primary weapon active.

Actual behaviour

• You spawn empty-handed, until you manually select your weapon.
• Bots also spawn empty-handed.

Additional context

Weapon selection behaviour before the bug

This is the behaviour before commit d209d44. Notice how the weapon is readied as soon as the player spawns, and the same holds true for bots:

wepselect1.webm

Weapon selection behaviour after the bug

Weapon does not become active after spawning, until the player manually selects it:

wepselect-regression-1.webm

Here's a third person view of a player spawning in empty-handed:

wepselect-regression-2.webm

Here's a bot spawning in empty-handed. There's also the T pose bug #569 occurring here, but that is unrelated to the bugs described here:

wepselect-regression-3.webm

Linked Issues

• related Player TPosing #569, Fix initial player tposing #570

[AdamTadeusz]

Would combining weapon_switch and ClientCommand work? Otherwise the first person view model muzzle flashes will be slightly scuffed when initially spawning
[Edit] if that pr gets merged in as well

[AdamTadeusz]

bots shoot again and my weapon is readied instantly so maybe just do both?

[Rainyan]

Would combining weapon_switch and ClientCommand work? Otherwise the first person view model muzzle flashes will be slightly scuffed when initially spawning [Edit] if that pr gets merged in as well

Could you clarify a bit? Any steps to reproduce the problem? I don't mind looking into it, but I've no idea what & where needs fixing.

[AdamTadeusz]

Would combining weapon_switch and ClientCommand work? Otherwise the first person view model muzzle flashes will be slightly scuffed when initially spawning [Edit] if that pr gets merged in as well

Could you clarify a bit? Any steps to reproduce the problem? I don't mind looking into it, but I've no idea what & where needs fixing.

on the viewmodel muzzleflash pr we update the properties of the muzzle flash when this function is called. Using weapon_switch this function is only called on the server side, when a client switches their own weapon with the slotx command the function is called both on the client and server.

Replace the ClientCommand on the first person view model muzzleflash branch with the original and then spawn in, select support and pick the supa7. The muzzle flash will be cross shaped instead of round and the size of the muzzle flash will be smaller than it should be

[Rainyan]

I see, thanks. There's a few problems with the approach.

Firstly, I'm worried it might create a race condition where the behaviour depends on whether the networked command reaches the client before or after they have materialized in the level. You could for example register a think context at CNEORules::PlayerSpawn for this, queuing it up for a future tick, to get around it.

But more critically, because we need to set the player's initial weapon as their primary slot server side, issuing a slot1 or similar command won't actually have an effect, because they already have that as their active item. You can verify this by calling slot<n> with that slot already equipped, and see how it doesn't fire CBasePlayer::Weapon_Switch client side.

I think this logic should be moved client side as much as possible. In this case, it's not really the switching of the weapon we're concerned about, but the fact the player has materialized in the word - so we could have the the client side player listening for the player_spawn event, and if it's their userid that fired it, do the whatever muzzle flash tweaks from there. This could be as simple as ListenForGameEvent("player_spawn") in C_NEO_Player ctor, and the corresponding FireGameEvent override:

void C_NEO_Player::FireGameEvent(IGameEvent *event)
{
	if (FStrEq(event->GetName(), "player_spawn"))
	{
		if (event->GetInt("userid") == GetUserID())
		{
			UpdateMuzzleFlashProperties(GetActiveWeapon());
		}
	}

	BaseClass::FireGameEvent(event);
}

But I think it's maybe outside the scope of this PR, and might be more suited as part of #559 , since it's the feature relying on this stuff.

[Rainyan]

Actually, looks like even the event listener fires too early (at least on LAN). Well, I'll mess around with a think based approach tomorrow.

[AdamTadeusz]

I merged in the view model muzzle flash PR, should make it easier to see what solutions we can come up with for this problem. Having the initial muzzle flash be a little broken is not a big problem so I suggest merging this in too