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

Move SelfActions from postInit to config #6791

Merged
merged 10 commits into from
Feb 24, 2019

Conversation

dedmen
Copy link
Contributor

@dedmen dedmen commented Jan 25, 2019

Also swapped around the condition on the create, it would do the expensive switch before doing the cheap isNil.

addons/zeus/CfgVehicles.hpp Outdated Show resolved Hide resolved
@dedmen dedmen changed the title Move zeus SelfActions to config Move SelfActions from postInit to config Jan 25, 2019
@Dystopian
Copy link
Contributor

I object.

  • postInit actions are much more flexible than config one. Config actions testing requires game restart (release, not dev) while postInit wants just mission restart;
  • config multiline code looks quite ugly and moving it to separate functions is overkill.

We save just some hundreds ms at mission start but I don't think it's really worth it. That's why I added both actions with postInit.

Maybe addActionToClass should be modified to speed up mission start instead. E.g. set some namespace variables to init actions on first using instead addClassEventHandler call.

@dedmen
Copy link
Contributor Author

dedmen commented Jan 25, 2019

postInit actions are much more flexible than config one. Config actions testing requires game restart (release, not dev) while postInit wants just mission restart;

Why does this action have to be flexible? Has nothing in there that one would want to change.

@Dystopian
Copy link
Contributor

Why does this action have to be flexible? Has nothing in there that one would want to change.

Any code can be improved. And it's easier to improve it if testing is simple.

@dedmen
Copy link
Contributor Author

dedmen commented Jan 25, 2019

Then improve it and put it into config when it's improved.

class ACE_SelfActions {
class GVAR(create) {
displayName = CSTRING(CreateZeus);
condition = QUOTE(\
Copy link
Contributor

Choose a reason for hiding this comment

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

14:09:37 File z\ace\addons\zeus\CfgVehicles.hpp, line 325: '/CfgVehicles/CAManBase/ACE_SelfActions/ace_zeus_create.condition': Missing ';' at the end of line
14:09:37 ErrorMessage: File z\ace\addons\zeus\CfgVehicles.hpp, line 325: /CfgVehicles/CAManBase/ACE_SelfActions/ace_zeus_create/: Wrong # directive
14:09:37 Application terminated intentionally
ErrorMessage: File z\ace\addons\zeus\CfgVehicles.hpp, line 325: /CfgVehicles/CAManBase/ACE_SelfActions/ace_zeus_create/: Wrong # directive

I think complex code like this should just be in a function, I hate dealing with ARR_X and string escaping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that mikeros tools? Might be a bug in his parser ^^
Yeah I'll move it.. Name?
zeus_fnc_actionCreateCondition ?

Copy link
Member

Choose a reason for hiding this comment

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

It's a condition, so zeus_fnc_canCreate (something with canX).

Copy link
Contributor

Choose a reason for hiding this comment

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

it was crashing arma on game start
the way it's now handled in master is good enough imho (https://github.com/acemod/ACE3/blob/master/addons/zeus/XEH_postInit.sqf#L85-L121)
otherwise zeus_fnc_actionCreateCondition/zeus_fnc_actionCreateStatement isn't bad because they are sorted alphabetically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code in current master will still execute code when you spawn units in Zeus right? Even if just once per type. Whereas the config one wouldn't even do that?
Also it seems like with the script code we'll be adding it to tons of CAManBase based classes, even though we really only need CAManBase once.

I checked the config code using the make script, Don't remember whether mikero or armake.

Copy link
Contributor

Choose a reason for hiding this comment

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

no code happens on object init
only fires once each time the player controls a new man/vehicle type

@PabstMirror PabstMirror added this to the 3.13.0 milestone Feb 4, 2019
@PabstMirror PabstMirror added the kind/cleanup Release Notes: **CHANGED:** label Feb 4, 2019
commy2 and others added 4 commits February 5, 2019 10:15
@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.6 Feb 24, 2019
@PabstMirror PabstMirror merged commit 48f45ae into acemod:master Feb 24, 2019
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
* Move zeus SelfActions to config

* Port CBA PR #1048

* Move Land_Camping_Light_off_F TurnOn action to config

* Move createModule function to script func

* Update addons/zeus/functions/fnc_canCreateModule.sqf

Co-Authored-By: dedmen <dedmen@users.noreply.github.com>

* Apply suggestions from code review

Co-Authored-By: dedmen <dedmen@users.noreply.github.com>

* Teaching it a lesson about spacing

* Update CBA: script_macros_common.hpp

* Just change admin in cba macro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Release Notes: **CHANGED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants