Add Feature Camera Player EH #982
Conversation
| ---------------------------------------------------------------------------- */ | ||
| SCRIPT(registerFeatureCamera); | ||
|
|
||
| GVAR(featureCameras) pushBackUnique _this; |
There was a problem hiding this comment.
Should probably add argument checking.
| @@ -192,6 +200,12 @@ if (_id != -1) then { | |||
| GVAR(oldCameraView) = _data; | |||
| [QGVAR(cameraViewEvent), [_player, _data]] call CBA_fnc_localEvent; | |||
| }; | |||
|
|
|||
| _data = call CBA_fnc_getActiveFeatureCamera; | |||
There was a problem hiding this comment.
performance?
We might want to only add this when it's actually needed.
There was a problem hiding this comment.
It's just a bunch of null checks (7 in vanilla). Aren't all Player EHs only added when they are needed already?
There was a problem hiding this comment.
0.0301 - 0.031 ms on 10k/10k cycles.
There was a problem hiding this comment.
0.03 ouhhhh.
How about a check whether either the array of eventhandlers is not empty (by using the CBA internal variable name of the array)
Or check that GVAR(oldFeatureCamera is not nil? and set it to non nil once a Eventhandler is added in line 92?
There was a problem hiding this comment.
That wouldn't work for cases where we might want to use internal variable (ref ACE PR).
|
https://github.com/CBATeam/CBA_A3/compare/camera-playerEH...featureCamOpti?expand=1 |
|
Comments on the commits, not the PR :x |
|
Well yes, they were made before that code was merged into this branch with the PR. How are you supposed to comment on the PR if it doesn't exist? |
|
@commy2 is this good or should it be completely split from |
| GVAR(oldFeatureCamera) = _data; | ||
| [QGVAR(featureCameraEvent), [call CBA_fnc_currentUnit, _data]] call CBA_fnc_localEvent; | ||
| }; | ||
| }] call CBA_fnc_compileFinal; |
There was a problem hiding this comment.
There is no reason to compileFinal this. It is only done for Map, because it is a mission event handler and therefore affected by the recompiled-every-execution-bug. That however does not apply to CBA PFH.
|
I have tested it, works perfectly fine with ACE3 (with acemod/ACE3#6573) and ACRE2 (with IDI-Systems/acre2#582). |
| @@ -214,6 +222,15 @@ if (_id != -1) then { | |||
| }; | |||
| } call CBA_fnc_directCall; | |||
| }; | |||
|
|
|||
| GVAR(playerEHInfo) pushBack ([{ | |||
There was a problem hiding this comment.
The handler is still always added even if noone is listening for it.
Seems to be fairly easy to just add it in L92 instead.
There was a problem hiding this comment.
We could do this since acemod/ACE3@922774d. Unsure how worth it is though, ACE3 is already set to use it and that covers a pretty big range of CBA users.
* master: Add Feature Camera Player EH (#982) German brands for JR and JAM components (#988) veteran29 Polish translation updates master (#986) German translation, part 3 (#981) German translation, part 2 (#980) German translation, part 1 (#979) add Joint Ammo Magazines A3 (#928) CBA_fnc_getVolume not calculating volume correctly (#984) ProjectileTracking - Stop tracking stationary objects (#985)
When merged this pull request will:
"featureCamera"Player Event Handler