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

UI - Fix selective UI in cargo #6585

Merged
merged 2 commits into from
Mar 11, 2019
Merged

Conversation

Dystopian
Copy link
Contributor

When merged this pull request will:

Maybe now VEHICLE_ONLY and GROUND_ONLY macros should be renamed e.g. to VEHICLE_NOT_CARGO and GROUND_AND_CARGO.

@jonpas
Copy link
Member

jonpas commented Sep 19, 2018

I don't like fade, it reminds me of slow fading away. Like vanilla option to fade after not being used, and show when needed again. Which is something Selective UI is missing btw.

@Dystopian
Copy link
Contributor Author

And I don't like show when it means not show. fade is for ctrlSetFade, nothing more.

@commy2
Copy link
Contributor

commy2 commented Sep 19, 2018

show and hide

[LSTRING(Disallowed), 2] call EFUNC(common,displayTextStructured);
false
};

_cachedElement params ["_idd", "_elements", "_location", "_conditions"];

// Exit if main vehicle type condition not fitting
private _canUseWeapon = ACE_player call CBA_fnc_canUseWeapon;
if ((_canUseWeapon && {_location == 2}) || {!_canUseWeapon && {_location == 1}}) exitWith {false};
private _cargoIndex = vehicle ACE_player getCargoIndex ACE_player; // nil if not in vehicle
Copy link
Contributor

Choose a reason for hiding this comment

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

Why check cargo specifically? Instead of just in vehicle check?

Copy link
Contributor Author

@Dystopian Dystopian Sep 19, 2018

Choose a reason for hiding this comment

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

because these controls (Soldier/Vehicle/Weapon Information, location = GROUND_ONLY) should be set when on foot or in vehicle cargo including FFV. See also #3877

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but why cargo? Why exclude driver/gunner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonpas should know. I think because that info is shown while player is in cargo and isn't shown when in other seats.

Copy link
Member

Choose a reason for hiding this comment

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

Why exactly does old way break with moveInCargo?

Copy link
Contributor Author

@Dystopian Dystopian Sep 19, 2018

Choose a reason for hiding this comment

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

Because ammocount is disabled when player is on foot and not updated when he gets in cargo.
When moveInCargo is used in init, player is never on foot at mission start and ammocount is not disabled because CBA_fnc_canUseWeapon returns false and location == GROUND_ONLY for ammocount.

Copy link
Contributor Author

@Dystopian Dystopian Sep 19, 2018

Choose a reason for hiding this comment

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

In fact I just added cargo seats to _canUseWeapon condition (on foot and FFV was there) and I had to rename it because _canUseWeapon name became wrong.

Copy link
Member

Choose a reason for hiding this comment

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

I see, I will test this soon.

@jonpas jonpas added this to the 3.13.0 milestone Oct 4, 2018
@jonpas jonpas added kind/enhancement Release Notes: **IMPROVED:** kind/bug-fix Release Notes: **FIXED:** labels Oct 4, 2018
private _canUseWeapon = ACE_player call CBA_fnc_canUseWeapon;
if ((_canUseWeapon && {_location == 2}) || {!_canUseWeapon && {_location == 1}}) exitWith {false};
private _cargoIndex = vehicle ACE_player getCargoIndex ACE_player; // nil if not in vehicle
private _isOnFootOrInCargo = isNil "_cargoIndex" || {_cargoIndex > -1};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private _isOnFootOrInCargo = isNil "_cargoIndex" || {_cargoIndex > -1};
private _isOnFootOrInCargo = isNil "_cargoIndex" || {_cargoIndex > -1} || {ACE_player call CBA_fnc_canUseWeapon};

Need this to handle non-cargo FFV turrets.
Example with merkel tank with commander gun,
inside it shows ammo (gunner ammo) fine
turn out and player's gun ammo shows, and exiting while in this state it will continue to show dismounted.

While turned out in turret::
ACE_player call CBA_fnc_canUseWeapon = false
vehicle ACE_player getCargoIndex ACE_player = -1

Could possibly also use isTurnedOut player, but cba func isn't that expensive

@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.6 Mar 10, 2019
@Dystopian
Copy link
Contributor Author

@PabstMirror I added non-cargo FFV check with variable renaming.

Also I'm not sure now if we should add more location types to config instead such GROUND_ONLY and FFV/cargo combination. Something like GROUND_FFV_CARGO etc.

@PabstMirror
Copy link
Contributor

I guess ground and vehicle aren't perfect, but I think the intent is clear enough
Ground = ground weapons, even if mounted
vehicle = vehicle's own weapons

) exitWith {
TRACE_3("skip location",_this,_canUseWeaponOrInCargo,_location);
false
};
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this whole thing like this:

if ((_canUseWeaponOrInCargo && {_location == VEHICLE_ONLY}) ||
    {!_canUseWeaponOrInCargo && {_location == GROUND_ONLY}}
) exitWith {
    TRACE_3("skip location",_this,_canUseWeaponOrInCargo,_location);
    false
};

Probably just because I am more used to it, either is mergable.

@PabstMirror PabstMirror merged commit 56f9d76 into acemod:master Mar 11, 2019
@Dystopian Dystopian deleted the fix_ui_ammocount branch March 11, 2019 05:29
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
* UI - Fix selective UI in cargo

* Add FFV not in cargo check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-fix Release Notes: **FIXED:** kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling Ammo Count interface - not affect player inside vehicles, if player uses - "this moveInCargo"
5 participants