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

Add Zeus utility modules #4661

Merged
merged 24 commits into from
Nov 15, 2016
Merged

Add Zeus utility modules #4661

merged 24 commits into from
Nov 15, 2016

Conversation

RSpeekenbrink
Copy link
Contributor

@RSpeekenbrink RSpeekenbrink commented Nov 12, 2016

When merged this pull request will:

  • Add a Zeus Module category ACE Util
  • Add Zeus Module: Toggle Simulation
    This module needs to be placed on an object. This will toggle the simulation of the object. Handy for base builds etc.
  • Add Zeus Module: Add Objects to Curator
    This module will ask for a radius once placed (radius in meters) and will add all objects to zeus within that radius.
  • Add Zeus Module: Remove Objects from Curator
    This module will do the same as Add but then inverted (Remove all objects in the radius from zeus, it will not remove the objects it will just make them uneditable and no icon will show up etc)

jonpas
jonpas previously requested changes Nov 12, 2016
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

Minor things required, no actual code checks performed.

@@ -644,6 +647,9 @@
<German>Fügt jedes gespawnte Objekt allen Kuratoren der Mission hinzu</German>
<Japanese>ミッション内で作成されたオブジェクトに全キュレーターを追加</Japanese>
</Key>
<Key ID="STR_ACE_Zeus_RemoveObjectsToCurator">
Copy link
Member

Choose a reason for hiding this comment

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

STR_ACE_Zeus_RemoveObjectsFromCurator

@@ -607,6 +607,9 @@
<Italian>Piazza su una unità</Italian>
<Japanese>ユニットの上に設置</Japanese>
</Key>
<Key ID="STR_ACE_Zeus_NoObjectSelected">
<English>Place on a object</English>
Copy link
Member

Choose a reason for hiding this comment

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

on an object

@@ -0,0 +1,64 @@
/*
* Author: Fisher
* Removes all objects in given radius for all curators
Copy link
Member

Choose a reason for hiding this comment

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

Missing final point.

* Removes all objects in given radius for all curators
*
* Arguments:
* 0: dummy controls group <CONTROL>
Copy link
Member

Choose a reason for hiding this comment

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

Dummy (capitalization)

* 0: dummy controls group <CONTROL>
*
* Return Value:
* none <NIL>
Copy link
Member

Choose a reason for hiding this comment

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

Just None.

*
* Return Value:
* None <NIL>
*
Copy link
Member

Choose a reason for hiding this comment

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

An empty line too many.

@@ -49,6 +52,14 @@ class ACE_Curator {
GVAR(cargoAndRepair)[] = {"ace_cargo", "ace_repair"};
};

class CfgFactionClasses {
Copy link
Member

Choose a reason for hiding this comment

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

Should be in separate file ˙CfgFactionClasses.hpp and included below.

@@ -49,6 +52,14 @@ class ACE_Curator {
GVAR(cargoAndRepair)[] = {"ace_cargo", "ace_repair"};
};

class CfgFactionClasses {
class ACE_Util {
Copy link
Member

Choose a reason for hiding this comment

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

ACE_UI_Util maybe?

PREP(moduleSurrender);
PREP(moduleTeleportPlayers);
PREP(moduleUnconscious);
PREP(moduleZeusSettings);
PREP(ui_attributeCargo);
//PREP(ui_attributePosition);
PREP(ui_AddObjects);
Copy link
Member

Choose a reason for hiding this comment

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

ui_addObjects - same for file.

PREP(ui_attributeRadius);
PREP(ui_defendArea);
PREP(ui_globalSetSkill);
PREP(ui_groupSide);
PREP(ui_patrolArea);
PREP(ui_RemoveObjects);
Copy link
Member

Choose a reason for hiding this comment

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

ui_removeObjects - same for file.

@jonpas jonpas added the kind/feature Release Notes: **ADDED:** label Nov 12, 2016
@jonpas jonpas added this to the 3.9.0 milestone Nov 12, 2016
commy2
commy2 previously requested changes Nov 12, 2016
Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

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

Newlines @ EOF.

Someone should test this in MP. With Zeus being a client. This heavily relies on global server only commands, but I'm pretty sure the way these modules are set up, they are executed on the zeus machine.

[LSTRING(NoObjectSelected)] call EFUNC(common,displayTextStructured);
} else {
_simulationEnabled = simulationEnabled _object;
_object enableSimulationGlobal (!_simulationEnabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't work in MP like this.
https://community.bistudio.com/wiki/enableSimulationGlobal

Has to be executed on server:
https://community.bistudio.com/wikidata/images/9/9f/Exec_Server.gif

Copy link
Member

Choose a reason for hiding this comment

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

There is an event in common component for it.

Copy link
Contributor Author

@RSpeekenbrink RSpeekenbrink Nov 12, 2016

Choose a reason for hiding this comment

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

Fixed it with BIS_fnc_MP now, I couldn't find the event you where talking about

Maybe its on me, been a while since I worked on ArmA files :P

#include "script_component.hpp"

params ["_logic"];
private ["_object","_simulationEnabled"];
Copy link
Contributor

Choose a reason for hiding this comment

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

use private keyword instead.


params ["_control"];
private _display = ctrlParent _control;
private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
Copy link
Contributor

Choose a reason for hiding this comment

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

objNull

};

private _fnc_onUnload = {
private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

{
_y = _x;
{ _x addCuratorEditableObjects [[_y], true]; } foreach allCurators;
} foreach _objectsToAdd;
Copy link
Contributor

Choose a reason for hiding this comment

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

forEach.
New scope in new line.
Has to be executed on server and probably doesn't work like this in MP.
missing private for _y

Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't this simply:

    {
        _x addCuratorEditableObjects [_objectsToAdd, true];
    } forEach allCurators;

addCuratorEditableObjects is already meant to be able to add arrays of objects. No point in walking over the array just to pass each element in a size one array.

};

private _fnc_onUnload = {
private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

private _display = ctrlparent _ctrlButtonOK;
if (isNull _display) exitWith {};

private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

{
_y = _x;
{ _x removeCuratorEditableObjects [[_y], true]; } foreach allCurators;
} foreach _objectsToAdd;
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@kymckay
Copy link
Member

kymckay commented Nov 12, 2016

Like the addition of module categories, have been meaning to get around to that myself 👍

UI stuff all looks to be handled correctly in line with the existing modules 👍 (I should probably post some explicit guidelines on standard procedure for the addition of new UI modules - I ought to fix that position attribute at some point too).

I'd like to see a setting on the add/remove modules to toggle whether it's all curators or just the current one (all by default), however I can easily add that after merge if you're not confident with the UI stuff.

Regarding the locality stuff:

Module functions will run globally by default (if inheriting from the base module), so they will run everywhere. The simulation module can thus just use the command without having to worry about locality (since it will be ran on all machines by the module framework).

The UI modules have their functions stem from the UI itself, so those are only run on the zeus client by default. You'll have to handle locality within those function (i.e running the add/remove object commands on the server).

@kymckay kymckay self-assigned this Nov 12, 2016
@@ -695,5 +701,8 @@
<Polish>Wpisano nieprawidłowy promień</Polish>
<German>Ungültiger Radius eingegeben</German>
</Key>
<Key ID="STR_ACE_Zeus_ModuleSimulation_DisplayName">
<English>Toggle Simulation</English>
</Key>
</Package>
</Project>
Copy link
Contributor

Choose a reason for hiding this comment

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

newline

[[_object, (!_simulationEnabled)],"enableSimulationGlobal",false] call BIS_fnc_MP;
};

deleteVehicle _logic;
Copy link
Contributor

Choose a reason for hiding this comment

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

newline


params ["_logic"];

if !(local _logic) exitWith {};
Copy link
Contributor

@commy2 commy2 Nov 12, 2016

Choose a reason for hiding this comment

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

@SilentSpike
So this function runs everywhere... Which machine is the module local to? Server or Zeus client? Shouldn't this be a isServercheck for simplicity regardless?

Copy link
Member

@kymckay kymckay Nov 12, 2016

Choose a reason for hiding this comment

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

Module will be local to zeus client. However in the case of this module, we may want to actually exploit the module framework and make it only run on the server (then use the simulationGlobal command).

To do so, in the module's cfgVehicles entry you simple set isGlobal = 0;

Copy link
Member

Choose a reason for hiding this comment

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

(and remove this locality check yes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then make the command instead of enableSimulationGlobal just enableSimulation ?

@RSpeekenbrink
Copy link
Contributor Author

Thanks SilentSpike for the comment, in a way I was confused about the Simulation command since I also thought if you call enableSimulationGlobal it will automatically be executed from the server to all clients. I just tested the simulation script on a server without the BIS_fnc_MP and it works fine.

About the UI stuff, i'm currently learning how UI stuff works in ArmA so im not that confident yet.

[LSTRING(NoObjectSelected)] call EFUNC(common,displayTextStructured);
} else {
private _simulationEnabled = simulationEnabled _object;
[[_object, (!_simulationEnabled)],"enableSimulationGlobal",false] call BIS_fnc_MP;
Copy link
Contributor

Choose a reason for hiding this comment

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

that is not what @jonpas and I meant. Should probably be restored to what it was if the other comment regarding the modules locality is solved.

{
_x addCuratorEditableObjects [_objectsToAdd, true];
} foreach allCurators;
} foreach _objectsToAdd;
Copy link
Contributor

Choose a reason for hiding this comment

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

that looks wrong


#include "script_component.hpp"

disableSerialization;
Copy link
Contributor

Choose a reason for hiding this comment

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

@SilentSpike
module scripts are run in scheduled environment, right?

Copy link
Member

Choose a reason for hiding this comment

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

These UI initialisation functions will stem from an onSetFocus event handler, which I don't believe is scheduled, not sure though.

Copy link
Contributor

Choose a reason for hiding this comment

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

If they don't run in scheduled environment, then disableSerialization should be removed as unscheduled environment does not support serialization to begin with.

{
_x removeCuratorEditableObjects [_objectsToAdd, true];
} foreach allCurators;
} foreach _objectsToAdd;
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above. no need to iterate through _objectsToAdd at all. also, forEach camel case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im a derp, I forgot to remove the other foreach loop

kymckay and others added 3 commits November 12, 2016 11:37
Module will run on the server only via the module framework (removing the need for any back and fourth).
# Conflicts:
#	addons/zeus/functions/fnc_moduleSimulation.sqf
@@ -22,7 +22,8 @@
[LSTRING(NoObjectSelected)] call EFUNC(common,displayTextStructured);
} else {
private _simulationEnabled = simulationEnabled _object;
[[_object, (!_simulationEnabled)],"enableSimulationGlobal",false] call BIS_fnc_MP;
_object enableSimulationGlobal (!_simulationEnabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

tab. should be four spaces to let travis pass

deleteVehicle _logic;

Copy link
Contributor

Choose a reason for hiding this comment

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

white space here

@commy2
Copy link
Contributor

commy2 commented Nov 12, 2016

@SilentSpike the same problem exists for fnc_ui_addObjects.sqf (addCuratorEditableObjects) and fnc_ui_removeObjects.sqf (removeCuratorEditableObjects

};

deleteVehicle _logic;

Copy link
Contributor

Choose a reason for hiding this comment

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

white space : (

RSpeekenbrink and others added 3 commits November 12, 2016 15:40
- Merges the add/remove curator editable objects modules into one with a checkbox to specific behaviour.
- Adds a checkbox to specify whether to effect all curators or only the local curator
- Fixes locality handling of the module code
@kymckay
Copy link
Member

kymckay commented Nov 12, 2016

I've merged the add/remove modules together, fixed their locality, added checkbox to toggle all/local curator.

All that's left now is some cleanup and this is good to go 👍

- Standardise function headers (remove examples where not applicable)
- Add module categories (and make display names beautiful)
- Alphabetise some classes
@kymckay kymckay dismissed jonpas’s stale review November 12, 2016 17:31

Changes have been met

@kymckay kymckay dismissed commy2’s stale review November 12, 2016 17:37

Changes have been met

@kymckay
Copy link
Member

kymckay commented Nov 12, 2016

Alright, I'm happy to merge this, but if someone else wants to give the code a quick re-review beforehand that'd be great 👍

@654wak654
Copy link
Contributor

I'm assuming #4597 is going to need category = QGVAR(Utility) under class moduleAddOrRemoveFRIES because of this PR?

All of @alganthe's PRs will need it too: #4555 #4556 #4576

@alganthe
Copy link
Contributor

Will modify my PRs when this is merged.

@commy2
Copy link
Contributor

commy2 commented Nov 12, 2016

lgtm. I'd use a macro for the IDC's,but that is a general problem with the module and goes beyond this PR.

}] call CBA_fnc_addEventHandler;

[QGVAR(removeObjects), {
params ["_objects", "_curator"];
Copy link
Contributor

Choose a reason for hiding this comment

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

@SilentSpike
Do you mean:
params ["_objects", ["_curator", objNull]];
so that the exitWith thing works as alternative syntax?
Alternativeley change !isNull _curator to !isNil "_curator"

same above

Copy link
Contributor

@commy2 commy2 Nov 12, 2016

Choose a reason for hiding this comment

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

I know that this works anyway (at least in unscheduled env.) due to isNull silently failing when _curator is undefined, but I still think relying on this SQF ... "feature" is bad design.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch commy, I don't know why I totally forgot params can supply a default value when writing this 👍


private _object = attachedTo _logic;
if (isNull _object) then {
[LSTRING(NoObjectSelected)] call EFUNC(common,displayTextStructured);
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 script does indeed only run on the server, then why does it show a text there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't it so that it runs on both server & local? @SilentSpike

Copy link
Member

Choose a reason for hiding this comment

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

Also a good catch, hmm, I suppose it probably should run on the client and then send an event to the server so that this client feedback can be given.

Copy link
Member

Choose a reason for hiding this comment

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

Not optimal in terms of minimising network traffic, but much better UX for zeus and also these modules are only running when prompted by zeus anyway so it's not a big deal.


params ["_control"];

disableSerialization;
Copy link
Contributor

@commy2 commy2 Nov 12, 2016

Choose a reason for hiding this comment

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

does nothing in unscheduled env.
should be removed if these module scripts do indeed run unscheduled. otherwise ignore this

Copy link
Member

Choose a reason for hiding this comment

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

This is probably my bad if unnecessary as it's in all of my existing ui functions, not sure if I had reasoning for that when I first wrote them

Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

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

see comments

@kymckay
Copy link
Member

kymckay commented Nov 13, 2016

@654wak654 @alganthe Modules don't necessarily need categories defined, if they inherit from the base module then it'll still default to ACE.

- Module must run on zeus client so that feedback can be shown if validation fails
- Use event in common to run the `enableSimulationGlobal` command
- Can use existing localised string
- Also adds default value to the editable objects events
@@ -15,11 +15,13 @@

params ["_logic"];

if !(local _logic) exitWith {};
Copy link
Contributor

Choose a reason for hiding this comment

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

so the module is local to the zeus? good to know

Copy link
Member

Choose a reason for hiding this comment

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

Yep, this line is standard in all zeus modules (BI included) with a few exceptions.

@kymckay
Copy link
Member

kymckay commented Nov 13, 2016

Just tested and confirmed that disableSerialization is unnecessary for the ui functions. Will remove that from them all while I'm at it.

if (isNull _object) then {
[LSTRING(NothingSelected)] call EFUNC(common,displayTextStructured);
} else {
[QEGVAR(common,enableSimulationGlobal), [_object, !(simulationEnabled _object)]] call CBA_fnc_serverEvent;
Copy link
Contributor Author

@RSpeekenbrink RSpeekenbrink Nov 14, 2016

Choose a reason for hiding this comment

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

Just for learning purposes, what does QEGVAR exactly in this state?
Thanks

Copy link
Member

@bux bux Nov 14, 2016

Choose a reason for hiding this comment

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

QuotedExternalGlobalVARiable
=> "ace_common_enableSimulationGlobal"

http://ace3mod.com/wiki/development/coding-guidelines.html#macro-usage

Copy link
Contributor

Choose a reason for hiding this comment

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

The line makes use of this event defined in the common component:
https://github.com/acemod/ACE3/blob/master/addons/common/XEH_postInit.sqf#L135

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aah I get it, makes a lot of sense now, thanks 👍

@kymckay kymckay changed the title Zeus Util Modules Add Zeus utility modules Nov 15, 2016
@kymckay kymckay merged commit 324bf68 into acemod:master Nov 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants