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 toggle flashlight and NVG zeus modules #4556

Merged
merged 14 commits into from
Sep 21, 2017

Conversation

alganthe
Copy link
Contributor

When merged this pull request will:

  • Add a module to toggle flashlights on or off for a group or a side.
  • Add a module to toggle NVGs on or off for a group or a side.

Both of those are added if they're not already present on the unit's weapon or head, it uses whatever the unit is supposed to have.
It works with headgear that has built in night vision / thermal.
If the units don't have flashlights on their weapons it checks if they have a compatible attachment that is a flashlight and give it to them (if none are available then so be it)

@alganthe alganthe changed the title Add toggle flashlight and NVG modules Add toggle flashlight and NVG zeus modules Oct 16, 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.

Didn't do full review, just quick skim and that caught my attention.

private _units = [];

if (_target == 4) then {
_units = units (attachedTo _logic);
Copy link
Member

Choose a reason for hiding this comment

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

Excessive space.

if (_target == 4) then {
_units = units (attachedTo _logic);
} else {
_units = allUnits select {alive _x && {side _x == ([blufor, opfor, independent, civilian] select _target)}},
Copy link
Member

Choose a reason for hiding this comment

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

Excessive space.

@jonpas jonpas added the kind/feature Release Notes: **ADDED:** label Oct 16, 2016
@jonpas jonpas added this to the 3.9.0 milestone Oct 16, 2016
jonpas
jonpas previously approved these changes Oct 16, 2016
@jonpas jonpas dismissed their stale review October 16, 2016 19:49

Solved, but not fully reviewed yet.

};

} else {
_x linkitem "NVGoggles";
Copy link
Member

Choose a reason for hiding this comment

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

linkItem

private _unit = effectiveCommander (attachedTo _logic);

scopeName "Main";
private _fnc_errorAndClose = {
Copy link
Contributor

Choose a reason for hiding this comment

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

this and the switch a few lines below is used in at least two places. Maybe make it a separate function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't do that due to scopeName being used, I could do that but other people writing curator modules would have to know that they have to define the scopeName before calling the function.

private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
if (isNull _logic) exitWith {};

private _combo1 = _display displayCtrl 56218;
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation


_control ctrlRemoveAllEventHandlers "setFocus";

// Handles errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Not meant for the code on line 32 right?

private _logic = GETMVAR(BIS_fnc_initCuratorAttributes_target,objnull);
if (isNull _logic) exitWith {};

private _combo1 = _display displayCtrl 92855;
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation

@alganthe
Copy link
Contributor Author

Made the changes required by the review above.

@kymckay kymckay self-assigned this Nov 15, 2016
@PabstMirror PabstMirror modified the milestones: 3.9.0, 3.10.0 Feb 11, 2017
@PabstMirror PabstMirror modified the milestones: 3.11.0, 3.10.0 Jun 2, 2017
Copy link
Contributor

@Phyma Phyma left a comment

Choose a reason for hiding this comment

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

Please fix header things :)

*
* Return Value:
* Nothing
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a example.

And return value is None

*
* Return Value:
* Nothing
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a example.

And return value is None

* 0: Nvg toggle controls group <CONTROL>
*
* Return Value:
* Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

Return Value: None

* 0: Flashlight toggle controls group <CONTROL>
*
* Return Value:
* Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

Return Value: None

private _pointer = (_x weaponAccessories (currentWeapon _x)) select 1;

if (!(_pointer isEqualTo "") && {isNull (configfile >> "CfgWeapons" >> _pointer >> "ItemInfo" >> "Pointer")}) then {
_x enableGunLights "ForceOn";
Copy link
Contributor

Choose a reason for hiding this comment

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

In https://community.bistudio.com/wiki/enableGunLights uses"forceOn" But I guess this is tested so it might still work :)

Copy link
Member

Choose a reason for hiding this comment

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

Probably isn't case-sensitive.

@jonpas
Copy link
Member

jonpas commented Jun 22, 2017

@alganthe Plan to work on that further, headers need some fixing. Aditionally, I think we should rather provide the option to give units gear, for cases when you want to enable NVGs/lights on a group but only certain units have them and you want only those enabled, not that every other unit gets them as well.

@alganthe
Copy link
Contributor Author

alganthe commented Sep 8, 2017

@jonpas that would only apply to flashlights, as NVGs are just put on by AI whenever they feel like it,.

@jonpas
Copy link
Member

jonpas commented Sep 8, 2017

Could use addPrimaryWeaponItem.

@jonpas jonpas modified the milestones: 3.11.0, Ongoing Sep 8, 2017
@alganthe
Copy link
Contributor Author

alganthe commented Sep 8, 2017

addWeaponItem also works with sidearms, where addPrimaryWeaponItem only works with the main slot.

@jonpas jonpas requested a review from kymckay September 17, 2017 22:29
@PabstMirror
Copy link
Contributor

enableGunLights requires a local unit acording to the biki
in testing on a dedicated on non-local units it sometimes worked and sometimes didn't,
probably want to use targetEvent for this

};

//Specific on-load stuff:
private _comboBox = _display displayCtrl 56218;
Copy link
Contributor

@PabstMirror PabstMirror Sep 19, 2017

Choose a reason for hiding this comment

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

Small usability improvement, should set the default to toggle their current state

private _enabledDefault = false;
if (!isNull _unit) then {
    _enabledDefault = !isFlashlightOn (leader _unit);
};
_comboBox lbSetCurSel [0,1] select _enabledDefault;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why leader _unit?
Don't you mean currentWeapon _unit?

@alganthe
Copy link
Contributor Author

there we go, indeed I didn't notice enableGunLights was AL.

if (local _x) then {
_x enableGunLights "forceOn";
} else {
[QGVAR(enableFlashlight), [_x, "forceOn"], _x] call CBA_fnc_targetEvent;
Copy link
Member

Choose a reason for hiding this comment

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

Target event handles locality on its own, so you don't need duplicate code. Probably minor performance difference, shouldn't matter here at all.

@@ -48,12 +52,11 @@ if (_toggle) then {

if (local _x) then {
_x addWeaponItem [(currentWeapon _x), _flashlight];
_x enableGunLights "ForceOn";
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@@ -62,7 +65,11 @@ if (_toggle) then {
} else {
{
if !(isPlayer _x || {(currentWeapon _x) isEqualTo ""}) then {
_x enableGunLights "ForceOff";
if (local _x) then {
_x enableGunLights "ForceOff";
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@bux
Copy link
Member

bux commented Sep 20, 2017

Nice, this will be a welcomed addition.

@@ -214,6 +214,18 @@ class CfgVehicles {
function = QFUNC(moduleUnconscious);
icon = QPATHTOF(UI\Icon_Module_Zeus_Unconscious_ca.paa);
};
class GVAR(moduleToggleNvg): GVAR(moduleBase) {
curatorCanAttach = 1;
category = QGVAR(Utility);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this rather be category AI? It specifically shouldn't work on players anyways (flashlight already doesn't).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't really know where to put it, good point.


if (_toggle) then {
{
if (hmd _x isEqualTo "") then {
Copy link
Member

Choose a reason for hiding this comment

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

Ignore if player?

@jonpas jonpas merged commit 1ba330e into acemod:master Sep 21, 2017
@kymckay
Copy link
Member

kymckay commented Sep 21, 2017

👍 Thanks for handling review @jonpas, good to see this finally in 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:** status/needs-testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants