netfox
netfox copied to clipboard
NetworkWeapon._can_fire() will desync if dependent on NetworkTime.tick
The example weapon script's implementation of _can_fire
relies on NetworkTime.tick
which will tend to be smaller when the client calls the function in their _tick
compared to when the server calls it in their _request_projectile
rpc.
Adding the line print(multiplayer.get_unique_id(), "] last_fire: ", last_fire, " -> ", NetworkTime.tick)
before assigning last_fire = NetworkTime.tick
confirms this:
626816780] last_fire_tick: 4050 -> 4052 ; 2 tick delay on the client
1] last_fire_tick: 4053 -> 4055 ; 2 tick delay on the server, but 1 tick behind the client
626816780] last_fire_tick: 4052 -> 4054 ; 2 tick delay on the client, but 1 tick ahead of server
626816780] last_fire_tick: 4054 -> 4055 ; Now a 1 tick delay on the client since _accept_projectile calls _after_fire
1] last_fire_tick: 4055 -> 4057 ; 2 tick delay on the server
; Server now gets the client's _request_projectile(id, 4054, {...}) rpc 4 ticks late
[ERR][netfox.extras::NetworkWeapon] Projectile ghisrnm0t4gq rejected! Peer 626816780 can't use this weapon now (tick=4054; NetworkTime.tick=4058)
This shows that the server is desynced from the client (inherently) and that delay isn't properly accounted for. I think the simplest solution would be to modify _can_fire()
, fire()
, and _after_fire()
to take the tick: int
from _request_projectile
(or an arbitrary if called locally from fire(tick: int)
), though that would break compatibility and require users to fix their _can_fire()
, fire()
, and _after_fire()
implementations. ~~Another option would be to temporarily store the tick: int
parameter in an externally accessible variable - potentially even in NetworkTime.tick
for maximum compatibility, which would need to be restored after the calls to _can_fire()
and _spawn()
. The downside here is that a player could potentially cheat by manipulating the tick
variable in their RPC request, but I think that should be handled by _is_reconcilable
.~~ EDIT: This doesn't work as it leads to projectiles being requested in the past. It seems the real solution involves removing the need to call both fire()
and _request_projectile
on the server (or at least confirm both of those calls are for the same projectile and skip the second one).