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 - Add ace_arsenal_fnc_saveLoadout as API to save loadouts #10151

Merged
merged 10 commits into from
Aug 10, 2024

Conversation

DartRuffian
Copy link
Contributor

When merged this pull request will:

  • Add a public function to save a given loadout

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}.

@DartRuffian
Copy link
Contributor Author

DartRuffian commented Jul 27, 2024

fnc_saveLoadout should have some more verification to make sure improper loadouts are not saved, but not entirely sure how that should be implemented.

I have also left the documentation (currently) unedited because I wasn't sure how that should be organized.

PR title is also open-ended in-case other functions come up.

@DartRuffian DartRuffian marked this pull request as draft July 27, 2024 05:44
@johnb432 johnb432 self-requested a review July 27, 2024 06:35
@LinkIsGrim
Copy link
Contributor

fnc_saveLoadout should have some more verification to make sure improper loadouts are not saved, but not entirely sure how that should be implemented.

arma-rs (#9015) might be able to handle that?

@DartRuffian
Copy link
Contributor Author

fnc_saveLoadout should have some more verification to make sure improper loadouts are not saved, but not entirely sure how that should be implemented.

arma-rs (#9015) might be able to handle that?

Works for me.
Just wasn't sure if verification should be handled here, since it's also handled when actually opening the loadouts menu

@DartRuffian
Copy link
Contributor Author

DartRuffian commented Jul 31, 2024

Should there be fnc_deleteLoadout and/or fnc_getLoadout at all?
Seems unnecessary as it's pretty easy to do by just pulling the variable and doing X.

If there's not a desire for other public functions, and verification is already handled, then this PR should be good then

@DartRuffian DartRuffian marked this pull request as ready for review August 2, 2024 22:07
@DartRuffian
Copy link
Contributor Author

DartRuffian commented Aug 2, 2024

Maybe the second param could also take a unit, and would use the unit's loadout if passed?

Was just something I thought of while at work, figured it might make saving loadouts a tiny bit easier for persistent campaigns or something similar.

["Loadout", [...]] call ace_arsenal_fnc_saveLoadout;

["Loadout", player, true] call ace_arsenal_fnc_saveLoadout;

@rautamiekka
Copy link
Contributor

Maybe the second param could also take a unit, and would use the unit's loadout if passed?

Was just something I thought of while at work, figured it might make saving loadouts a tiny bit easier for persistent campaigns or something similar.

["Loadout", [...]] call ace_arsenal_fnc_saveLoadout;

["Loadout", player, true] call ace_arsenal_fnc_saveLoadout;

100% agree.

@johnb432
Copy link
Contributor

johnb432 commented Aug 5, 2024

Was just something I thought of while at work, figured it might make saving loadouts a tiny bit easier for persistent campaigns or something similar.

Imo not worth the extra code inside the functions, you can more easily add player call CBA_fnc_getLoadout to this function's call. It would be yet another thing to have to maintain for BWC, in case we ever want to change something in the future.

@DartRuffian
Copy link
Contributor Author

Fair point, should be good for review then.

Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>
Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Documentation still required.

@DartRuffian
Copy link
Contributor Author

Was waiting to see if anything else might pop up. I'll write it after work (~7 hours)

@DartRuffian
Copy link
Contributor Author

Added a scripting example, I don't think an entire section is needed for a relatively simple function.

@johnb432 johnb432 added the kind/feature Release Notes: **ADDED:** label Aug 10, 2024
@johnb432 johnb432 added this to the 3.18.0 milestone Aug 10, 2024
@johnb432 johnb432 changed the title Arsenal - Add more public functions Arsenal - Add ace_arsenal_fnc_saveLoadout as API to save loadouts Aug 10, 2024
@johnb432 johnb432 merged commit e36363e into acemod:master Aug 10, 2024
5 checks passed
blake8090 pushed a commit to blake8090/ACE3 that referenced this pull request Aug 18, 2024
…cemod#10151)

* Added fnc_saveLoadout

* Changed to toLower for other languages

* GitHub didn't like editing the file in the browser

* Fix case-sensitive _loadoutIndex

Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>

* Unicode support

Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>

* setVariable in case no loadouts are saved

* Fix return not happening properly

* Added scripting example

* Update docs/wiki/framework/arsenal-framework.md

---------

Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
@DartRuffian DartRuffian deleted the arsenal/public-functions branch September 23, 2024 00:55
@rautamiekka
Copy link
Contributor

Now is too late, but set (and some other commands) supports using -1 to indicate the end of an array since 'Arma 3 v2.12', so, unless there's something I dunno, it shouldn't be necessary to do a separate pushBack for -1.

@DartRuffian
Copy link
Contributor Author

-1 is not an index in this case, findIf returns -1 if the given item is not found in the array.

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

Successfully merging this pull request may close these issues.

5 participants