Skip to content

fix: register fired handlers on all clients simultaneously - #124

Merged
fank merged 10 commits into
OCAP2:mainfrom
thegamecracks:thegamecracks/firedman-sync
Aug 4, 2026
Merged

fix: register fired handlers on all clients simultaneously#124
fank merged 10 commits into
OCAP2:mainfrom
thegamecracks:thegamecracks/firedman-sync

Conversation

@thegamecracks

@thegamecracks thegamecracks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

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?):

  1. Start a dedicated server with OCAP and have a mission already running (-autoinit + persistent=1)
  2. Connect to the server and login as admin during the lobby screen (assuming skipLobby=0)
  3. Join the mission in progress and local exec the following in debug console:
    _id = player getVariable "OCAP_firedManEH";
    [player getVariable "OCAP_firedManEHExists",_id, player getEventHandlerInfo ["FiredMan",_id]]
  4. Check that the result is [<null>,<null>,[true,true,1]] indicating that the event handler failed to register
  5. Start a recording and fire a weapon anywhere
  6. (Optional) Respawn and/or return to lobby and repeat 3-4 until handler is registered, then fire again
  7. End recording and view in playback that projectiles (bullets + non-bullets) are not recorded without the EH

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:

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)

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...

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 fank left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Unit exists server-locally from mission start → class EH init fires on the server → local _entity branch adds EHs on the server only.
  2. Player joins → locality transfers server→client.
  3. Server's "Local" EH fires with _isLocal = false and 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_addClassEventHandler registers only on the executing machine and XEH init fires per-machine, so broadcasting the registration with retroactively = true + JIP gives every machine its own EHs and makes locality transfers a non-event.
  • The !local guard in FiredMan is necessary, not just cautious — FiredMan does fire on remote units within visibleFire/audibleFire camera 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_init is isServer-gated and runs once, so exactly one JIP message is queued; server-initiated remoteExec is exempt from CfgRemoteExec restrictions; HCs now correctly get EHs for HC-local AI via target 0 (an improvement); SHOULDSAVEEVENTS works on clients since recording/startTime are publicVariable'd; the Zeus remote-control swap in eh_fired_client still 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.

Comment thread addons/recorder/fnc_eh_fired_server.sqf
Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
@thegamecracks
thegamecracks requested a review from fank August 3, 2026 19:16
@thegamecracks

Copy link
Copy Markdown
Contributor Author

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
fank previously requested changes Aug 3, 2026

@fank fank left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
Comment thread addons/recorder/fnc_eh_fired_server.sqf
Comment thread addons/main/config.cpp
Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
@thegamecracks
thegamecracks marked this pull request as draft August 3, 2026 19:47
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.
@thegamecracks
thegamecracks marked this pull request as ready for review August 3, 2026 21:54
@thegamecracks
thegamecracks requested a review from fank August 3, 2026 21:55
@fank
fank dismissed their stale review August 3, 2026 22:00

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 fank left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread addons/recorder/fnc_eh_fired_server.sqf
Comment thread addons/recorder/fnc_eh_fired_server.sqf Outdated
@fank
fank merged commit f4050b2 into OCAP2:main Aug 4, 2026
1 check passed
@thegamecracks
thegamecracks deleted the thegamecracks/firedman-sync branch August 4, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants