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

Wire cutter - Allow use when unit has RHS Engineer UMBTS backpack #6462

Merged
merged 2 commits into from Jul 27, 2018

Conversation

Projects
None yet
4 participants
@Dystopian
Contributor

Dystopian commented Jul 22, 2018

When merged this pull request will:

  • title.

2

Class name rhs_assault_umbts_engineer is hard-coded on purpose because I didn't find any analogues in vanilla, RHS and CUP gear.

@Blutze

This comment has been minimized.

Contributor

Blutze commented Jul 22, 2018

Gives it a 5 mass capacity advantage over standardised Assault Packs. Considering most of that is carried on the outside, I'd be ok with that.

Not sure what to think about the precedent of adding functionality to other mods simply based on cosmetic aspects of their models though. And how well does that code work when you don't have RHS loaded? Does it still work if it is replaced with some kind of macro in case other examples crop up?

#define HAS_WIRECUTTER(unit) (\
"ACE_wirecutter" in ([ARR_6(unit, false, true, true, true, false)] call CBA_fnc_uniqueUnitItems) \
|| {backpack unit isKindOf "rhs_assault_umbts_engineer"} \

This comment has been minimized.

@dedmen

dedmen Jul 22, 2018

Contributor

Backpacks that inherit from this are not guaranteed to have a model with a wirecutter on it.

This comment has been minimized.

@PabstMirror

PabstMirror Jul 22, 2018

Collaborator

I would rather see something config based like
1 == getNumber (configFile >> "CfgVehicles" >> (backpack unit) >> QGVAR(hasWirecutter))
instead of hard coded classes

This comment has been minimized.

@Dystopian

Dystopian Jul 22, 2018

Contributor

@dedmen Yes. Right now there are 3 backpacks with this model. I decided not to check model in backpack config (which can also be changed in any RHS update) but just check inheritance. I think it is enough for now. If you insist I'll change the check.

This comment has been minimized.

@Dystopian

Dystopian Jul 22, 2018

Contributor

@PabstMirror I would implement it if there was at least very small probability for other such backpacks.

@Dystopian

This comment has been minimized.

Contributor

Dystopian commented Jul 22, 2018

And how well does that code work when you don't have RHS loaded?

It works OK. I thought about init checking if RHS is loaded and then replacing code with just false but I found it superfluous.

Does it still work if it is replaced with some kind of macro in case other examples crop up?

If there are more examples it would be very easy to add their support. Until then there is no sense to do it.

@PabstMirror PabstMirror added this to the 3.12.3 milestone Jul 22, 2018

@Blutze

This comment has been minimized.

Contributor

Blutze commented Jul 23, 2018

I would implement it if there was at least very small probability for other such backpacks.

@jokoho48 mentioned VSM and another mod that even had them on vests. Haven't touched either in ages, but even if there isn't anything in those - I'd still try and keep this future proof.

@@ -59,5 +59,6 @@
#define HAS_WIRECUTTER(unit) (\
"ACE_wirecutter" in ([ARR_6(unit, false, true, true, true, false)] call CBA_fnc_uniqueUnitItems) \
|| {backpack unit isKindOf "rhs_assault_umbts_engineer"} \
|| {1 == getNumber (configFile >> "CfgVehicles" >> (backpack unit) >> QGVAR(hasWirecutter))} \
|| {1 == getNumber (configFile >> "CfgWeapons" >> (vest unit) >> QGVAR(hasWirecutter))} \

This comment has been minimized.

@dedmen

dedmen Jul 27, 2018

Contributor

Vests are CfgVehicles.

This comment has been minimized.

@PabstMirror

PabstMirror Jul 27, 2018

Collaborator

They are Items/CfgWeapons.
There is a vestContainer command, that returns the vest "object"
but typeOf vestContainer player = "Supply140"

This comment has been minimized.

@PabstMirror PabstMirror changed the title from Allow wirecutter use when unit has RHS Engineer UMBTS backpack to Wire cutter - Allow use when unit has RHS Engineer UMBTS backpack Jul 27, 2018

@PabstMirror PabstMirror merged commit a44411c into acemod:master Jul 27, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Dystopian Dystopian deleted the Dystopian:rhs_wirecutter branch Jul 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment