Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: master
Are you sure you want to change the base?
Remote-Promise system #1414
Changes from 3 commits
af5b472
fbd6d48
1bc3176
fed80b4
4a2d77f
49f3c9e
3a950e7
7024e75
a2cea8a
ac912c9
a673d2b
6164de7
e2068d1
5129a5d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 conditionThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 thecreate
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.There was a problem hiding this comment.
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:
Ugly. Ternaries are bad. Good Code should be linear, not branched:
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.