CBATeam / CBA_A3

Community Base Addons for Arma 3
https://github.com/CBATeam/CBA_A3/wiki
GNU General Public License v2.0
364 stars 151 forks source link

Events - Add `CBA_fnc_addBISPlayerEventHandler` for adding EHs to just the player (from ACE3) #1670

Closed LinkIsGrim closed 1 month ago

LinkIsGrim commented 3 months ago

When merged this pull request will:

~I'm not sure if the code block below is equivalent, since there's no ACE_player here.~ It isn't.

if (_ignoreVirtual && {(unitIsUAV focusOn) || {getNumber (configOf focusOn >> "isPlayableLogic") == 1}}) exitWith {};
PabstMirror commented 3 months ago

I think we can replace ace_player with call CBA_fnc_currentUnit iirc focusOn might be funny when controlling a uav, not sure

commy2 commented 3 months ago

Just a few thoughts.

I don't personally like CBA_fnc_addPlayerAction and always thought of it as legacy code. I would always recommend to simply add user actions (and by extension event handlers) to every unit and then to quit early via guard clause. Is the mental overhead required for using such an obscure framework justified considering one could simply write:

if (_unit != player) exitWith {};

?

LinkIsGrim commented 3 months ago

This is only ever intended for internal use, to replace the polling in player events (hence, no removal option) and so the mental overhead is a one-time thing during implementation IMO. I don't like the idea of adding the EHs to every unit, due to the overhead (though it might still be better than polling). unit playerEH doesn't fire that often (basically only on respawn) for a non-curator on the other hand, and that's already laggy, so it doesn't matter.

I can make this exit on canSuspend, but again, this should only ever be called by CBA itself a few times at the start of a mission.

PabstMirror commented 3 months ago

There is some performance savings by only adding events you need. e.g. FiredNear could almost be considered n^2 based on unit count

We can make it safe by wrapping in a CBA_fnc_directCall; like https://github.com/CBATeam/CBA_A3/blob/master/addons/events/fnc_addEventHandler.sqf

LinkIsGrim commented 3 months ago

Implemented as public function. Added removal, thread-safety, and multiple events of the same type can be added now.

Do I need to touch anything myself for docs?