Player whose weapons are locked (for spawn killing) can receive 'weapons unlocked' message but then get 'weapons locked for 1 second' when trying to fire

[handsm]

Player get weapons locked. Trying to fire during this time results in red screen message and buzzer sound. Weapons then unlock and player gets white 'weapons unlocked' screen message. Player tries to fire straight away (say within next second) and gets a confusing 'weapons locked for 1 second' red screen message and buzz, when he expects to be able to fire.

Only in multiplayer, as this is caused by a difference between the server and client versions of elapsed time, which are not quite in sync. When locking weapons the server sets the time when they will be unlocked. This is specified in the round's ElapsedTime, being whole seconds from round start, i.e. it determines what time the weapons are due to be unlocked. This number is passed to the player's client.

The problem is the client's ElapsedTime can easily be a second behind the server and the client's time is used to determine whether it allows the player to fire weapons. The server would also check and would allow firing, but the client stops the signal even being sent to the server. Also the client's time is used to calculate the 'weapons locked for X seconds' messages, so those messages are out of sync with the server.

[handsm]

A number of options/ thoughts on this:

Move all weapon lock timing to the client. Functionality on the client is more vulnerable, but this is not especially critical gameplay functionality, i.e. a player managing to hack the client functionality would gain minimal benefit, perhaps none.

Pass the client (i.e. replicate) a number of seconds for weapons to be locked, instead of the time when weapons are due to be unlocked. Client could then set own local unlock time, based on its own ElapsedTime.

A simple clientside hack so when player received the unlocked message, their unlock weapons time is changed to be the current time (or zero or a negative, but no longer in the future). This means the client unlocks locally as soon as the server tells it the weapons are unlocked.

Stopping the 'weapons locked for X seconds' message from being displayed if there is only 1 second remaining. Stops the confusing/nonsensical weapons locked message just after a weapons unlocked message.

[AndrewTheel]

I'd say #2 or #3 are the better options that don't sound like it will require much rework of the current system. But this is Basnett's thing now, I just wrong a crappy foundation which he rewrote to be better.

[handsm]

Ok, as a first stage I've done an 'as is' (meaning keeps to existing code structure, for now) re-factor of the weapons lock system in commit 3eeb27b.

Digging into this system a little more, it's a bit of a mish-mash of clientside and server checks anyway. The process is initiated on the server, as it has to be because it starts in server-only damage/killing functionality. But from then on it's actually the client that prevents firing by using this unlock time. The server appears to control unlocking weapons, in DHGame's 1 second repeating Timer(), but all that is actually doing is send the client an unlock message. The control over firing is all from the unlock time, which the client has and uses to stop firing. There ought to be independent server verification as a back up. For infantry weapons there is, but probably only because the server happens to call the same ReadyToFire() function as is used by the client. For vehicle weapons and mortars there is no server check.

Switching to a replicated client function to handle the weapon lock on the client will solve a lot of issues and allow better/extra functionality, with less replication. I've worked it all though so will commit in stages.

[handsm]

Fixed in commit 4fb28c0.

Server now calls a replicated client function to set weapons lock on client, passing no. of seconds in that function. Unlocked time is no longer a replicated variable. Client now sets unlock time from its own ElapsedTime time plus no. of received lock seconds. Client handles its own locked message without need for server to separately replicate message broadcast.

Unlock message is now handled by client, without need for server to check or to replicate the unlock message function. So unlock timing is now fixed and no chance of contradictory unlocked then locked messages. Client's timing may be slightly behind server, due to delay in receiving replicated function, but functions generally get replicated straight away so delay will be minimal. But no problem anyway as client won't let player fire until its own local lock time has expired, so doesn't matter if server treats player as unlocked a split second earlier. bWeaponsAreLocked now becomes a clientside flag instead of a server flag, as its now the client that needs to know it need to give an unlock message.

Server still maintains verification of any firing attempt, although that's only a back up and in practice client will prevent firing, based on its own timing.

Clientside timing is now handled by the local GRI's repeating 1 second Timer(), which is the clientside equivalent of the server version in DHGame. DHGame's Timer() still checks the unlocks for a local player, which applies to single player or a hosting listen server player, as the GRI timer only runs on a client.
But DHGame now only needs to check a local player and no longer has to loop through all controllers.
As the checks in both timers are identical, they call a new CheckUnlockWeapon() function in DHPlayer, which further consolidates the core functionality into DHPlayer.

LockWeapons() is now a simulated function that runs on both the server and client (on client it gets called by the replicated ClientLockWeapons() function).

Moved the functionality that forces player to 'release' the fire buttons when weapons are locked or if he tries to fire while locked. Took it out of the message class and added it to LockWeapons() and AreWeaponsLocked(), but only for a local player. Works exactly the same, but its primary purpose/benefit is to stop automatic weapon fire, with avoiding spammed repeating messages & buzz sounds as secondary benefit, so it's more appropriate (and direct) to have it in those functions.

[handsm]

For reference, in commit cefde2f I had a re-think and changed the forced 'release' of the fire button from the AreWeaponsLocked() function to the message class. The initial fire button 'release' is done in LockWeapons() and that is to stop automatic fire from continuing - it is highly relevant to have that where it is. But after the initial weapon lock, the forced fire button release is purely to stop the player holding down the button and receiving continuous spammed warning message/sounds. So it is actually very appropriate and direct to have that 2nd release button functionality in the message class.

[handsm]

Re-opened as changes has been corrupted in merge between master and squad branches and it will no longer work in multiplayer. Just need to add back and delete a coupe of things. Will commit soon, when separated out other code work.

[handsm]

Corrupted merge fixed in commit c232855.