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

CSW/Mk6 Mortar - Make Mk6 Mortar Ammo Handling override CSW's #9238

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Jul 2, 2023

When merged this pull request will:

  • Title.
  • Change conditions on CSW ammo handling: actions will only be shown if both ammo handling is enabled and weapon assembly mode is true.
  • Prevent CSW from unloading weapon magazines if Ammo Handling is disabled.
  • Remove proxyWeapon function from Mk6 Mortar. Not needed anymore, mag replacement is handled correctly by CSW.
  • Document disabled assembly mode effect.
  • Update English and Portuguese stringtables to match new behavior.

Partially addresses #8866 and #7477.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@LinkIsGrim LinkIsGrim requested a review from PabstMirror July 2, 2023 19:16
@LinkIsGrim LinkIsGrim added the kind/enhancement Release Notes: **IMPROVED:** label Jul 9, 2023
@LinkIsGrim LinkIsGrim changed the title CSW - Add variable to disable for individual weapons CSW/Mk6 Mortar - Make Mk6 Mortar Ammo Handling override CSW's Jul 13, 2023
// If magazine handling is enabled or weapon assembly/disassembly is enabled we enable ammo handling
if ((GVAR(ammoHandling) == 0) && {!([false, true, true, GVAR(defaultAssemblyMode)] select (_target getVariable [QGVAR(assemblyMode), 3]))}) exitWith { false };
// If magazine handling and weapon assembly/disassembly are enabled we enable ammo handling
if ((GVAR(ammoHandling) == 0) || {!([false, true, true, GVAR(defaultAssemblyMode)] select (_target getVariable [QGVAR(assemblyMode), 3]))}) exitWith { false };
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this,
before even if ammoHandling was off you could still always load a "CSW" static weapon

e.g. if you assemble a csw from 2 carried weapons then it would still be loadable
because otherwise you just have a empty weapon that you can't do anything with

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll figure something out. Though you might agree that ammo handling still being possible despite the setting is confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I'd probably say it's mission-maker error
but docs also say

### 3.3 ammoHandling

- Whether or not you want to handle ammo using the CSW way. Does nothing if using defaultAssemblyMode
- Default: On

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO it shouldn't even be an option. This feature makes no sense without ammo handling.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should revert it to the previous behaviour, despite the setting being confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you split off the changes not related to this specific PR into a separate one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants