Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote-Promise system #1414

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions addons/promise/$PBOPREFIX$
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x\cba\addons\promise
11 changes: 11 additions & 0 deletions addons/promise/CfgFunctions.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class CfgFunctions {
class CBA {
class Network {
PATHTO_FNC(promise_create);
PATHTO_FNC(promise_callback);
PATHTO_FNC(promise_done);
PATHTO_FNC(promise_receiver);
PATHTO_FNC(promise_init);
};
};
};
17 changes: 17 additions & 0 deletions addons/promise/config.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include "script_component.hpp"

class CfgPatches {
class ADDON {
author = "$STR_CBA_Author";
name = CSTRING(component);
url = "$STR_CBA_URL";
units[] = {};
weapons[] = {};
requiredVersion = REQUIRED_VERSION;
requiredAddons[] = {};
version = VERSION;
authors[] = {"X39"};
};
};

#include "CfgFunctions.hpp"
29 changes: 29 additions & 0 deletions addons/promise/fnc_callback.sqf
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_promise_callback

Description:
Method 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!
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


Parameters:
_id - The Promise-ID of the now finished promise.
_data - The result data of the now finished promise.

Returns:
Nothing

Example:
Nothing

Author:
X39
---------------------------------------------------------------------------- */
#include "script_component.hpp"

params ["_id", "_data"];
private _request = GVAR(requests) select _id;
GVAR(requests) set [_id, 0];
[_request select 0, _data] call (_request select 1);
nil
51 changes: 51 additions & 0 deletions addons/promise/fnc_create.sqf
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_promise_create

Description:
Creates and executes a new Promise.

Parameters:
_objects - Object or array of objects that perform Say <OBJECT, ARRAY>
_params - [sound, maxTitlesDistance,speed] or "sound" <ARRAY, STRING>
X39 marked this conversation as resolved.
Show resolved Hide resolved

_rcv - remoteExecCall target.
_mthd - either string (methodname) or code to execute on target.
_mthdArgs - custom arguments to pass to the target.
_args - args to pass to the callback (see next param)
_cb - callback for the promise (executed on current machine).
X39 marked this conversation as resolved.
Show resolved Hide resolved
Will receive an array, with index 0 being what is passed include
_args and index 1 being whatever is returned by the target invokation
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling "invocation".

[_args, _mthdResult]

Returns:
Identifier that allows to check if a promise is yet done.
See CBA_fnc_promise_done.

Example:
(begin example)
[
random_player, "random_method", [],
[_someLocalVariable], {
params ["_args", "_result"];
_args params ["_someLocalVariable"];
Copy link
Contributor

Choose a reason for hiding this comment

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

rogue space

diag_log _result;
diag_log _someLocalVariable;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this were

systemChat str [_result];

the example would show me a result in game.

}
] call CBA_fnc_promise_create;
(end)

Author:
X39
---------------------------------------------------------------------------- */
#include "script_component.hpp"
params ["_rcv", "_mthd", "_mthdArgs", "_args", "_cb"];
private _stamp = diag_tickTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be noted then that this framework breaks on servers that run for 11 days.

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

private _index = 0;
isNil {
_index = GVAR(requests) find 0;
if (_index == -1) then
{ _index = GVAR(requests) pushBack [_args, _cb, _stamp]; }
else { GVAR(requests) set [_index, [_args, _cb, _stamp]]; }
X39 marked this conversation as resolved.
Show resolved Hide resolved
};
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

[_index, _mthd, _mthdArgs] remoteExec [QGVAR(requests), _rcv, false];
[_index, _stamp]
24 changes: 24 additions & 0 deletions addons/promise/fnc_done.sqf
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_promise_done

Description:
Checks if a given promise was processed yet.

Parameters:
_promiseHandle - Handle returned by CBA_fnc_promise_create
X39 marked this conversation as resolved.
Show resolved Hide resolved

Returns:
Boolean, true if it is done yet, false if it is not.
X39 marked this conversation as resolved.
Show resolved Hide resolved

Example:
(begin example)
_promiseHandle call CBA_fnc_promise_done;
(end)

Author:
X39
---------------------------------------------------------------------------- */
#include "script_component.hpp"
params ["_index", "_stamp"];
private _promise = GVAR(requests) select 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it get a promise from a list of requests?
Should it be _request or GVAR(promises) instead?

if (_promise isEqualTo 0) { true } else { _promise select 2 != _stamp }
X39 marked this conversation as resolved.
Show resolved Hide resolved
28 changes: 28 additions & 0 deletions addons/promise/fnc_init.sqf
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_promise_init

Description:
Initializes the required variabels for the promise system.
X39 marked this conversation as resolved.
Show resolved Hide resolved

Parameters:
Nothing

Returns:
Nothing

Example:
(begin example)
[] call CBA_fnc_promise_init;
(end)

Author:
X39
---------------------------------------------------------------------------- */
#include "script_component.hpp"
isNil {
if isNil QGVAR(requests) then {
GVAR(requests) = [];
};
};

nil
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

I am not aware of how to realize that with the macros in CfgFunctions.hpp PATHTO_FNC(promise_init);

Copy link
Contributor

Choose a reason for hiding this comment

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

Just put it in XEH preInit.

Copy link
Contributor

@commy2 commy2 May 12, 2021

Choose a reason for hiding this comment

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

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.

32 changes: 32 additions & 0 deletions addons/promise/fnc_receiver.sqf
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_promise_receiver

Description:
Method that gets called when a sender has a request.

WARNING! Not intended to be called by client-code!

Parameters:
_id - the promise ID, local to the sender.
_mthd - The method to execute, either string (var name) or code.
_args - Arguments to pass to the method.
X39 marked this conversation as resolved.
Show resolved Hide resolved

Returns:
Nothing

Example:
Nothing

Author:
X39
---------------------------------------------------------------------------- */
#include "script_component.hpp"

params ["_id", "_mthd", "_args"];
X39 marked this conversation as resolved.
Show resolved Hide resolved
private _res = if (_mthd isEqualType "") then {
_args call (missionNamespace getVariable _mthd);
} else {
_args call _mthd;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

[_id, _res] remoteExec [QGVAR(requests), remoteExecutedOwner, false];
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

What do you suggest as alternative?

Copy link
Contributor

Choose a reason for hiding this comment

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

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
21 changes: 21 additions & 0 deletions addons/promise/license.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
MIT License

Copyright (c) 2018 Marco Silipo (X39)

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
4 changes: 4 additions & 0 deletions addons/promise/script_component.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#define COMPONENT promise
#include "\x\cba\addons\main\script_mod.hpp"

#include "\x\cba\addons\main\script_macros.hpp"
X39 marked this conversation as resolved.
Show resolved Hide resolved