Remote-Promise system #1414
Remote-Promise system #1414
Conversation
| } else { | ||
| _args call _mthd; | ||
| }; | ||
| [_id, _res] remoteExec [QGVAR(requests), remoteExecutedOwner, false]; |
There was a problem hiding this comment.
Last time I checked, the RE target id and the return value of REOwner do not match up in loaded local hosted multiplayer games, so this is not safe.
| ---------------------------------------------------------------------------- */ | ||
| #include "script_component.hpp" | ||
| params ["_rcv", "_mthd", "_mthdArgs", "_args", "_cb"]; | ||
| private _stamp = diag_tickTime; |
There was a problem hiding this comment.
Should be noted then that this framework breaks on servers that run for 11 days.
There was a problem hiding this comment.
diag_tickTime is only used to get some value that changes enough to allow reusage of the same slots and checking in isDone. As long as it changes enough, it is fine. However, i may also rework it into a simple counter that counts up. That however could introduce a race condition
|
It bugs me that the function names are CBA_fnc_promise_xyz and not cba_promise_fnc_xyz Would prefer either PREP or camel case: CBA_fnc_promiseXyz |
| }; | ||
| }; | ||
|
|
||
| nil |
There was a problem hiding this comment.
This function seems like bloat to me. Just run this in preInit and tell people to add requiredAddons if they really need this in preInit as well.
There was a problem hiding this comment.
I am not aware of how to realize that with the macros in CfgFunctions.hpp PATHTO_FNC(promise_init);
|
Why does it constantly talk about "methods"? Shouldn't it be "functions" or is this specifically a thing about promises I am missing. |
Can alter that. There is nothing you are missing here. |
Which one of the two would you prefer? |
Please do.
Camel case, since PREP is not used much for public functions due to the advantage of the functions viewer. |
| @@ -26,7 +26,7 @@ SCRIPT(findMax); | |||
|
|
|||
| [_this] params [["_array", [], [[]]]]; | |||
|
|
|||
| if !(_array isEqualTypeAll 0) exitWith {nil}; | |||
| if !(_array isEqualTypeAll 0) exitWith { nil }; | |||
| _args call (missionNamespace getVariable _function); | ||
| } else { | ||
| _args call _function; | ||
| }; |
There was a problem hiding this comment.
This control flow is bad:
params ["_id", "_function", "_args"];
private _res = if (_function isEqualType "") then {
_args call (missionNamespace getVariable _function);
} else {
_args call _function;
};Ugly. Ternaries are bad. Good Code should be linear, not branched:
params ["_id", "_function", "_args"];
if (_function isEqualType "") then {
_function = missionNamespace getVariable [_function, {}];
};
private _res = _args call _function;Much better.
Dunno about the implications of reading arbitrary globals as functions and calling them. Probably a lot of evil that can be done with that.
| } else { | ||
| _args call _function; | ||
| }; | ||
| [_id, _res] remoteExec [QGVAR(requests), remoteExecutedOwner, false]; |
There was a problem hiding this comment.
remoteExec [QGVAR(requests)
Does this even work? Seems to me that requests is an array, but RE is for function names/STRINGs. I may be stupid again though.
We don't use RE in CBA or ACE, but events instead.
| }; | ||
| }; | ||
|
|
||
| nil |
There was a problem hiding this comment.
This whole function should be nuked and the default of requests should be defined either in XEH PreInit, or the create function should make this value if undefined.
The benefit of preInit is one less if-operation per function call (neglible honestly). The benefit of defining it on demand is that this makes create work in preInit for addons that do not require cba_main or this component specifically.
| ---------------------------------------------------------------------------- */ | ||
| #include "script_component.hpp" | ||
| params ["_index", "_stamp"]; | ||
| private _promise = GVAR(requests) select 0; |
There was a problem hiding this comment.
Why does it get a promise from a list of requests?
Should it be _request or GVAR(promises) instead?
| random_player, "random_function", [], | ||
| [_someLocalVariable], { | ||
| params ["_args", "_result"]; | ||
| _args params ["_someLocalVariable"]; |
| params ["_args", "_result"]; | ||
| _args params ["_someLocalVariable"]; | ||
| diag_log _result; | ||
| diag_log _someLocalVariable; |
There was a problem hiding this comment.
If this were
systemChat str [_result];the example would show me a result in game.
| ---------------------------------------------------------------------------- */ | ||
| #include "script_component.hpp" | ||
| params ["_receiver", "_function", "_functionArgs", "_args", "_callback"]; | ||
| private _stamp = diag_tickTime; |
There was a problem hiding this comment.
This is an absolute no go and doesn't even guarantee that there are no collisions, especially after diag_tickTime exceeded the floating point precission after 11 days. This should be a counter, possibly as string prepended with the client id (CBA_clientOwner should be a safe way to get it) to make this work in mp.
| else { | ||
| GVAR(requests) set [_index, [_args, _callback, _stamp]]; | ||
| } | ||
| }; |
There was a problem hiding this comment.
CBA generally does not require thread safety, because writing thread safe code is a pipe dream. If a function should be atomic regardless, for example if it is supposed to be API used by the scrubs/mission makers, then we/(I?) just use this construct instead:
https://github.com/CBATeam/CBA_A3/blob/master/addons/settings/fnc_set.sqf#L27-L30
No reason to shit up the code for the scheduler.
| Function that gets called when the receiver is done. | ||
| Will in the end execute the promise-code locally. | ||
|
|
||
| WARNING! Not intended to be called by client-code! |
There was a problem hiding this comment.
Pretty sure we have a Public: No doc token for this very purpose. It's also another argument to define these functions using PREP() instead of CfgFunctions, because generally only API is exposed to CfgFunctions. See the settings-component, where the CfgFunctions functions are just wrappers for the internal PREP()ed functions with some preInit handling.
Just noticed that i have had this "in the pipe" since 2018 ... finally creating the PR for it
When merged this pull request will: