fix: register fired handlers on all clients simultaneously - #124
Conversation
Clients sometimes didn't receive their FiredMan / HandleDamage event handlers when joining-in-progress or respawning. This addresses what I suspect to be a race condition with the CBA class event handler not tracking projectiles from players. This is my theory of the order of operations when a client joins: 1. Player unit spawns on server locality 2. Player unit transfers locality to client 3. CBA fires the class EH If (2) and (3) were switched around, the class EH may see the player unit as local before it's transferred, and would be unable to remote execute fired handlers on the client. After locality is transferred, the Local EH removes the server's EHs from the unit, but does not tell the client to set up its own fired handlers. As a result, the player unit is left with no EH to track their projectiles. If my theory is correct, a solution here is to make every client set up the same handlers on every unit. FiredMan and HandleDamage have locality documented here: - https://community.bistudio.com/wiki/Arma_3:_Event_Handlers#FiredMan - https://community.bistudio.com/wiki/Arma_3:_Event_Handlers#HandleDamage For FiredMan, the event can sometimes trigger on remote units if within proximity to the player's camera, but otherwise only fires reliably where the unit is local. For HandleDamage, the event handler should never on remote units. In both events, I added guard conditions to both out of caution to ensure the EHs only trigger on the respective client. An alternative solution could be to keep the original server-side class EH, but tweak Local EH to remote execute adding/removing handlers whenever locality shifts. However, the Local EH is expected not fire on the server when locality transfers between two clients, such as when the group leader changes: https://community.bistudio.com/wiki/Arma_3:_Event_Handlers#Local_(Entity) > The event handler only triggers on the computers that are directly involved > in change of locality. So if EH is added to every computer on network, > it will only trigger on 2 computers, on the computer that receives ownership > of the object ... and on the computer from which ownership is transferred ... As such, I believe it is more reliable to have all clients register their event handlers and let it fire on whomever the unit is local to. This theory does have one contradiction from a separate wiki page: https://community.bistudio.com/wiki/Arma_3:_Mission_Event_Handlers#OnUserSelectedPlayer > [OnUserSelectedPlayer] is the earliest the player object is known when > player joins the server, but it is not local to the user yet, so there > is a wait time depending on network connection. When player respawns, > **the unit created on the client** and so it might take a while before > server has valid player object. Why was the EH registration flaky even when respawning repeatedly, if the unit is supposedly created on the client to begin with? I don't have an answer for this...
fank
left a comment
There was a problem hiding this comment.
Thanks a lot for the thorough investigation and write-up — the diagnosis is correct, and I verified it against the BIKI/CBA docs. One correction that actually makes your case stronger than the PR description states:
Root cause (confirmed)
The old code's comment claimed "The Local EH is global (ironically) when applied to a unit" — that assumption is false. addEventHandler has local effects (EL), and the "Local" EH only fires on machines that (a) added the EH and (b) are directly involved in the transfer. So the "Local" EH added by the server existed only on the server.
For a JIP into an existing slot (-autoinit + persistent=1) this wasn't even a race — it failed deterministically:
- Unit exists server-locally from mission start → class EH
initfires on the server →local _entitybranch adds EHs on the server only. - Player joins → locality transfers server→client.
- Server's
"Local"EH fires with_isLocal = falseand removes the server EHs; the client never had a"Local"EH, so nothing ever adds handlers on the new owner. Result: zero FiredMan EHs anywhere.
Your open question (respawn flakiness)
On respawn the unit is created client-side, but the server's XEH init for the remote proxy fires during the creation/networking window — at that moment owner _entity can still report 2 (server) or 0, or the object isn't fully networked yet, so remoteExec ["call", owner _entity] either targeted the wrong machine or the object deserialized as objNull on arrival (the exact hazard the old owner-0 comment warned about). Timing-dependent → flaky, and repeated respawns eventually won the race.
Why the new approach is sound (verified)
CBA_fnc_addClassEventHandlerregisters only on the executing machine and XEHinitfires per-machine, so broadcasting the registration withretroactively = true+ JIP gives every machine its own EHs and makes locality transfers a non-event.- The
!localguard in FiredMan is necessary, not just cautious — FiredMan does fire on remote units withinvisibleFire/audibleFirecamera range; without it nearby players would double-report. - HandleDamage only fires where the unit is local, so that guard is defensive-only — fine.
- Sanity checks that pass:
fnc_initisisServer-gated and runs once, so exactly one JIP message is queued; server-initiatedremoteExecis exempt fromCfgRemoteExecrestrictions; HCs now correctly get EHs for HC-local AI via target0(an improvement);SHOULDSAVEEVENTSworks on clients sincerecording/startTimearepublicVariable'd; the Zeus remote-control swap ineh_fired_clientstill works since the Zeus avatar is local to the controlling client.
Requesting changes for two small items (inline): a nil-CBA guard on clients and updating the now-misleading file header. Two further inline comments are non-blocking nits. Core fix is solid — happy to approve once those two are addressed.
This is in line with README.md which already documents the required version as 2.10. Given that Arma is already at 2.20, maybe it should be bumped to that?
|
For requiredVersion, any thoughts on bumping it even higher to 2.20? I imagine OCAP doesn't need to support older Arma versions, and would allow HEMTT to pass off on newer commands and events. |
fank
left a comment
There was a problem hiding this comment.
Re-reviewed after the latest 5 commits — this got even better. The native EntityCreated reimplementation removes the client-side CBA dependency for registration entirely, and I verified the semantics: EntityCreated is a per-machine mission EH that also fires for remote/replicated entities as they are created on each machine (the BIKI even notes it fires "when remote entity is created" before the variable namespace copy on respawn — irrelevant here since we read no variables at add time). Combined with the retroactive forEach allUnits and the JIP-queued broadcast, coverage is equivalent to the CBA class-EH approach. The file header is updated too — thanks!
One small but real logic issue remains (inline, at the CBA guard), which contradicts your own stated intent from the earlier thread; plus two non-blocking observations. Happy to approve once the guard is sorted.
EntityCreated fires on respawning units which preserve their event handlers. Adding the handler without checking would cause projectile events to incorrectly fire multiple times. This makes two changes: 1. Check for existing handlers before adding them 2. Wait one frame before adding handlers on remote units The second point is to address this note in the wiki: https://community.bistudio.com/wiki/Arma_3:_Mission_Event_Handlers#EntityCreated > This event is called before variable namespace is copied to remote respawning entity, > keep this in mind so your setVariables are not overwritten in the next frame. > EntityRespawned always fires after variable namespace is copied to new entity > regardless of locality.
Required change (CBA guard exitWith) was addressed in 8f28e71, and the requiredVersion question was answered — EntityCreated itself requires 2.10. Dismissing; follow-up review incoming for the new duplicate-handler commit.
fank
left a comment
There was a problem hiding this comment.
Third pass, covering 8f28e71, bf390c5 and 1046f6d. The CBA guard is fixed exactly as intended (HandleDamage now registers without CBA, inner FiredMan guard is meaningful), the dangling comment is reworded, and the requiredVersion bump is fully justified — EntityCreated itself is a 2.10 MEH, so my earlier 2.04 remark was off-target; I've dismissed the previous review.
The new duplicate-prevention guard in 1046f6d is well-motivated, but it rests on one engine-behavior claim I couldn't confirm from the BIKI, and there's one scenario where — if that claim doesn't hold on remote machines — the original bug quietly returns. One verification request + one nit inline. Once the respawn+locality-transfer case is confirmed working, this gets my approval; everything else looks great.
Clients sometimes didn't receive their FiredMan / HandleDamage event handlers when joining-in-progress or respawning. This addresses what I suspect to be a race condition with the CBA class event handler not tracking projectiles from players.
Steps to reproduce (maybe this should be an issue on the main repo?):
-autoinit+persistent=1)skipLobby=0)[<null>,<null>,[true,true,1]]indicating that the event handler failed to registerThis is my theory of the order of operations when a client joins:
If (2) and (3) were switched around, the class EH may see the player unit as local before it's transferred, and would be unable to remote execute fired handlers on the client. After locality is transferred, the Local EH removes the server's EHs from the unit, but does not tell the client to set up its own fired handlers. As a result, the player unit is left with no EH to track their projectiles.
If my theory is correct, a solution here is to make every client set up the same handlers on every unit. FiredMan and HandleDamage have locality documented here:
For FiredMan, the event can sometimes trigger on remote units if within proximity to the player's camera, but otherwise only fires reliably where the unit is local. For HandleDamage, the event handler should never on remote units. In both events, I added guard conditions out of caution to ensure the EHs only trigger on the respective client.
An alternative solution could be to keep the original server-side class EH, but tweak Local EH to remote execute adding/removing handlers whenever locality shifts. However, the Local EH is expected not fire on the server when locality transfers between two clients, such as when the group leader changes: https://community.bistudio.com/wiki/Arma_3:_Event_Handlers#Local_(Entity)
As such, I believe it is more reliable to have all clients register their event handlers and let it fire on whomever the unit is local to.
This theory does have one contradiction from a separate wiki page: https://community.bistudio.com/wiki/Arma_3:_Mission_Event_Handlers#OnUserSelectedPlayer
Why was the EH registration flaky even when respawning repeatedly, if the unit is supposedly created on the client to begin with? I don't have an answer for this...