[ISSUE-9451] Loadout Management

[FenikSRT4]

Partially Fixes #9451

Description of the problem being solved:

• Adds Loadout management replacing the single New Loadout option in the loadout dropdown. This new Manage option opens a ListControl menu for creating new, copying, renaming and deleting whole loadouts. This does not change the rules around managing the names for each set that's part of a loadout. For example, renaming an item set still disassociate the loadout.
• Multiple functions that existed in the list controls were refactored to be functions that were part of the respective tab class. ie: Renaming is now a function in ItemsTab as a class function instead of that logic living in the list control
• Several refactors around builds were pulled out of creating the list into functions so they could be reused or called elsewhere.

Testing Needed

• Renaming loadouts
• Copying loadouts
• Deleting loadouts
• Creating new loadouts
• Selecting a loadout from the new list control

Before screenshot:

After screenshot:

Loadout Dropdown - New Manage option replacing New Loadout option

Loadout Management List Control

Rename Loadout

Copy Loadout

General Notes

I'm not proficient in lua nor this project. If there's any guidance for documentation in this project to update along with these changes or lua best practices, that would be much appreciated.

[Nightblade]

Hey there, thanks for the contribution! Great idea making a Loadout Manager.

There are some guidelines in CONTRIBUTING.md but from my very quick glance at your code I did notice a mix of space and TAB indentation — please use TABs only.

[Nightblade]

BTW there is a dev Discord, so drop me a DM (nighty_b) if you would like an invite.

[FenikSRT4]

BTW there is a dev Discord, so drop me a DM (nighty_b) if you would like an invite.

I sent a friend request! The suffix should look familiar

[FenikSRT4]

Hey there, thanks for the contribution! Great idea making a Loadout Manager.

There are some guidelines in CONTRIBUTING.md but from my very quick glance at your code I did notice a mix of space and TAB indentation — please use TABs only.

Understood on the tab/space preference. I'll get that updated tomorrow

[Peechey]

Some issues I'm seeing:

1. Open new build, click Manage in dropdown, close it, click Manage again, nothing happens. The dropdown selIndex isn't getting reset/changing so it doesn't process again. Easy fix.

2. Open new build, try to Copy Loadout "Default", error. Probably a check needed for (... title or "Default")

3. Doesn't have "create new loadouts from existing sets" logic. I've seen this requested multiple times and I'd say it pretty much required if we're introducing more functionality here. In my PR it's a copy of the currently active sets.

4. New build, make a New Loadout, delete the new (2nd) Loadout. It deletes the Tree but breaks on either Skill/Item/Config

Lastly, for a lot of these cases and flows, we really need to have tests for them. I'm a bit of a hypocrite as my PR only has basic ones now, but the plan was to beef those up after working on Imbued Supports. Loadouts are complex, they get messy fast, and they have kind of high vis due to build creators using them in guides.

[FenikSRT4]

@Nightblade Thanks for pointing that out. It doesn't seem to be showing as an issue for EmmyLuaCodeStyle. Likely because it's recognizing it as a string.

Do either of you mind setting the wip label? I can re-tag y'all for a review when it's ready.

[FenikSRT4]

@Peechey I'm not yet able to request reviewers. Functionally, I think I've covered all of the current edge cases and I've added tests around all of those that I could find.

I think I'd like to clean up and refactor some of the new functions I've added in this because it feels pretty messy still.

Doesn't have "create new loadouts from existing sets" logic. I've seen this requested multiple times and I'd say it pretty much required if we're introducing more functionality here. In my PR it's a copy of the currently active sets.

If I'm understanding this request correctly, this sounds like the Copy Loadout functionality. The only difference would be a new passive tree? But even then from a usability standpoint, I think it would be simple enough to respec the tree from the starting point of another loadout. I'd like to try without this request and get feedback from users before adding this functionality because it feels a bit redundant.

[Nightblade]

space/mixed indents

[Peechey]

Looking good and I haven't been able to break it! ⭐⭐⭐⭐⭐

Regarding the Existing New, I may be twisting implementations in my head so call me out if this doesn't make any sense: the issue/lack with Copy is that you can only select a single loadout. I want the ability to copy/make a new loadout that consists of sets from different loadouts.

For example, I have 4 loadouts named Loadout 1, Loadout 2, Loadout 3, Loadout 4. I want to create/copy from existing Loadout 5 that has the tree from Loadout 1, the itemSet from Loadout 2, the skillSet from Loadout 3, and the configSet from Loadout 4. I could go to each tab and manually copy the set I want and name it Loadout 5, but I want to do it all at once. So I would like to be able to make those sets active, click Loadout Dropdown > Manage > New > Create New from Active Sets with name "Loadout 5". It should do the same thing but with a third? a quarter? as many clicks and without having to enter the name multiple times.

There might be a better way to solve this, not involving a Create New From Existing option, but I haven't been very good explaining this particular solution yet, so I wanna make sure we understand the functionality first.

[FenikSRT4]

@Peechey Your explanation does make sense to me now and I do see how it would be valuable. I'll play around to see if I can come up with a multi-list option that would allow users to chose from a single UI rather than tabbing through multiple.