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

Arsenal - Fix #9040 bugs/regressions #9293

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Jul 28, 2023

When merged this pull request will:

  • Fix script error when saving a loadout with a weapon in a container.
  • Readd magazine refill when clicking on equipped magazine in arsenal.

Missed while testing #9040.

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 added kind/bug-fix Release Notes: **FIXED:** ignore-changelog Release Notes: Excluded labels Jul 28, 2023
if (_forEachIndex in [4, 5]) then {
_uniqueBaseCfgText = (getText (_cfgMagazines >> _x >> QGVAR(uniqueBase))) call EFUNC(common,getConfigName);
_x params ["_magazine"];
_uniqueBaseCfgText = (getText (_cfgMagazines >> _magazine >> QGVAR(uniqueBase))) call EFUNC(common,getConfigName);

if (_uniqueBaseCfgText != "") then {
_weaponsInfo set [_forEachIndex, _uniqueBaseCfgText];
Copy link
Contributor

Choose a reason for hiding this comment

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

is it a problem that we lose the ammo count?

should this be something like

_weaponsInfo set [_forEachIndex, [_uniqueBaseCfgText, _x # 1]];
``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It still works and refills the magazine. I don't think it's a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested saving a loadout and reloading it?

I'm have a feeling that this fix can't work, as I had to write this for weapons not in containers. getUnitLoadout returns weapons in containers in the same format as weapons not in containers, so I presume the code should be the same and I forgot to correct that in #9040.

I'll take a in-depth look tonight.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's as I thought unfortunately.

I fixed it in #9297.

@LinkIsGrim LinkIsGrim changed the title Arsenal - Fix saving loadout with weapon in container Arsenal - Fix #9040 bugs/regressions Jul 28, 2023
@LinkIsGrim LinkIsGrim merged commit 12905a0 into acemod:master Jul 28, 2023
5 checks passed
@jonpas jonpas added this to the 3.16.0 milestone Jul 28, 2023
@johnb432
Copy link
Contributor

Readd magazine refill when clicking on equipped magazine in arsenal.

Makes sense for most use cases. However, if the magazine is unique, do we want that?

@LinkIsGrim
Copy link
Contributor Author

Readd magazine refill when clicking on equipped magazine in arsenal.

Makes sense for most use cases. However, if the magazine is unique, do we want that?

You can already refill unique mags by switching to and from any other magazine anyway. We'd have to keep the magazine's ammo count and restore it if we want consistency.

@johnb432 johnb432 mentioned this pull request Jul 28, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-changelog Release Notes: Excluded kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants