Skip to content

Events - Improve CBA_fnc_globalEventJIP: handle object as jipID#1777

Merged
PabstMirror merged 5 commits intoCBATeam:masterfrom
OverlordZorn:globalEventJIP
Jul 7, 2025
Merged

Events - Improve CBA_fnc_globalEventJIP: handle object as jipID#1777
PabstMirror merged 5 commits intoCBATeam:masterfrom
OverlordZorn:globalEventJIP

Conversation

@OverlordZorn
Copy link
Copy Markdown
Contributor

When merged this pull request will:
Added handling for Objects as JIP:
=> Remove JIP once object is deleted

Copy link
Copy Markdown
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Not sure if having the _jipID serve two different purposes is a good idea: Imagine you only put _obj as parameter, when you meant to write hashValue _obj - makes debugging much harder.
Personally I think a new separate parameter is more suited for this.

@OverlordZorn
Copy link
Copy Markdown
Contributor Author

OverlordZorn commented Jun 29, 2025

Not sure if having the _jipID serve two different purposes is a good idea: Imagine you only put _obj as parameter, when you meant to write hashValue _obj - makes debugging much harder. Personally I think a new separate parameter is more suited for this.

I get your point - my motivation was to mirrorhow remoteExec works for the custom jipID but also Objects with pretty much the same effect as here.

Happy to adjust it in a more suitable way. I could rename the params variable to sth like _jipOrObj and resolve it into 2 loc vars internally

@OverlordZorn OverlordZorn requested a review from johnb432 July 1, 2025 14:53
Copy link
Copy Markdown
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Just making sure you are aware: My reviews don't mean much in CBA, I'm only a contributor and not a maintainer.

[QGVAR(eventJIP), [_eventName, _params]] call CBA_fnc_globalEvent;

// Remove JIP once obj is deleted
if (!isNil "_obj") then { [_jipID, _obj] call FUNCMAIN(removeGlobalEventJIP); };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Despite it being technically correct, I wouldn't use the FUNCMAIN macro as it's not used anywhere else in CBA.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!isNil "_obj") then { [_jipID, _obj] call FUNCMAIN(removeGlobalEventJIP); };
if (!isNil "_obj") then { [_jipID, _obj] call CBA_fnc_removeGlobalEventJIP };

params [["_eventName", "", [""]], ["_params", []], ["_jipID", "", [""]]];
params [["_eventName", "", [""]], ["_params", []], ["_jipOrObj", "", ["", objNull]]];

private ["_obj", "_jipID"];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally, I hate this way of declaring variables in sqf.

@OverlordZorn
Copy link
Copy Markdown
Contributor Author

Just making sure you are aware: My reviews don't mean much in CBA, I'm only a contributor and not a maintainer.

fair. just if anyone is interacting with the PR's i am very happy to adjust simply to ensure chances of successful merge.

OverlordZorn and others added 2 commits July 7, 2025 16:42
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
@PabstMirror PabstMirror added this to the 3.18.4 milestone Jul 7, 2025
@PabstMirror PabstMirror merged commit 9dec36c into CBATeam:master Jul 7, 2025
4 checks passed
@OverlordZorn OverlordZorn deleted the globalEventJIP branch July 8, 2025 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants