Uh oh!
There was an error while loading. Please reload this page.
[ISSUE-9451] Loadout Management - #9647
Conversation
* 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. * 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.
Nightblade
commented
Mar 16, 2026
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
commented
Mar 16, 2026
BTW there is a dev Discord, so drop me a DM (nighty_b) if you would like an invite. |
FenikSRT4
commented
Mar 16, 2026
I sent a friend request! The suffix should look familiar |
FenikSRT4
commented
Mar 16, 2026
Understood on the tab/space preference. I'll get that updated tomorrow |
* Formatted changed code
Peechey
commented
Mar 16, 2026
Some issues I'm seeing:
 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. |
* Adds basic tests for NewLoadout, CopyLoadout, DeleteLoadout, and RenameLoadout
Uh oh!
There was an error while loading. Please reload this page.
FenikSRT4
commented
Mar 18, 2026
@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. |
* Linting spaces to tabs
* Resolves a nil title bug for the copy functionality * Resolves a loadout control selection when the list control is opened and immediately closed
* Added a new service class to relay loadout logic * Started adding tests for the service class with the hope of replicating some errors seen in the front-end * Some more linting for spaces to tabs
* Adds a new service class to facilitate all critical functions for the loadout management. This class allows the abstraction of that logic away from the UI class, BuildSetListControl, which makes the logic testable. * Adds tests around the new BuildSetService including integration style tests based on edge cases from manual UI testing. * Resolved various bugs from edge cases around Copying and Deleteing loadouts
FenikSRT4
commented
Mar 29, 2026
@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.
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. |
* Verifies that the modflag is being set appropriately for the build when changes happen
Uh oh!
There was an error while loading. Please reload this page.
Peechey
commented
Mar 30, 2026
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
commented
Mar 30, 2026
@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. |
* Linting for the skills tab changes
* Added new control dialog for creating a new loadout from a selection of the sets. This new option named New/Copy Custom opens a dialog with a loadout name buffer and 4 drop down selectors. Each selector has an option of New and the currently available sets. If a loadout was selected before clicking on this button, the dropdowns are prefilled with the selected sets, with the intention that they each would be copied. It otherwise defaults to the New options, as a redundancy for a new loadout. * Refactored and expanded the BuildSetListControl to allow room for this new button. * The save logic for this new dialog has yet to be implemented.
* Adds a new function to the BuildSetService for copying or creating a new loadout from user input selections * Adds tests to test this behavior
* Updates the BuildSetListControl to call the new CustomLoadout function
FenikSRT4
commented
Apr 4, 2026
@Peechey I've implemented a new dialog based on my interpretation of the ask and how I can best think to solve it now. Take a look and see if anything breaks. In the meantime I'll continue with refining the solution and testing it out. |
* Refactored the BuildSetService CustomLoadout implementation to relay to buildMode and orchestrate the syncing and selection
* Removed the callback function from NewLoadout and RenameLoadout because it wasn't consistent with the other lifecycle funtion implementations
* Refactored SkillTab CRUD operations and SkillSetListControl CRUD controls to be similar to the config versions * Added tests * Updated build to use the new CRUD controls
* Added ItemsTab CRUD functions * Added ItemsSetService class * Added Tests for Items tab and Item Set Service * Updated Build to utilize the new functions and made appropriate updates for testing
* Small bugfixes from UI testing * Ensures that all set controls are syncing loadouts as expected * Ensures that skills are being imported appropriately after the refactoring of NewSkillSet to CreateSkillSet * Updates PassiveSpecListControl to copy a passive spec appropriately
* Test coverage for the broken states that could occur when deleting the last skill set
* Added a new global for storing the filtered loadout list to be used within the BuildSetListControl to be consistent with the drop down * Refactored ReorderLoadout callback to reorder the spec list so when a sync occurs, the loadout list is updated to the expected output.
* Updated tests to use loadoutsList instead of speclist to be consistent with the list control
* Cleanup of unused variables.
* Refactored ReorderLoadout to recreate specList based on the reordered loadoutList
* Added tracking for activeLoadout. * Resolves issue where opening the management popup would not display the current loadout * Closing the popup now automatically selects the active loadout again.
* Updated GetLoadoutByName to return nil for fields that don't exist. This affects the cases of attempting to delete a loadout when there's only a single config, item, or skill set. Before it would delete that set and the respective tab would recreate a default one. This felt pretty bad given the user is likely making use of the one set feature. * Updated CopyLoadout to copy the single set if there's only one. This was a side-effect of the previous change
* Fixes a bad name concatenation for specs when copying a tree * Adds tests for CopyTree
* Readded t_insert because it was incorrectly removed
* Added a lookup to prevent loadouts with duplicative names. This should discourage users from doing so, which could set up scenarios of inconsistent behavior given the name is the primary key for lookup
* Updated GetLoadoutByName to work with oneSet scenarios again * Updated the loadoutsList to seed from the filteredSpecs given the specList and the treeList were getting desynced when linkIdentifier builds were in the mix.
* Restored default title behavior to Rename and Copy that had regressed after refactoring
* Restored Tree Management copy functionality * Implemented a default name for the New Tree button * Set a default name for the spec title for the case of renaming or copying the default spec.
* Provide default "Default" title fallback when copying config sets with nil title * Provide default "Default" title fallback when copying item sets with nil title * Set newly created loadout as active immediately after creation in Build.lua
Fix items getting deleted from shared loadouts If an item belonged to 2 loadouts and 1 of them was deleted, it would wipe the items without checking to see if another loadout used them Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Pressing F2 to rename the loadout now grabs the right name Renaming the loadout by pressing F2 would grab a table address instead of the name Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Importing a shared item list fix When importing a shared item list it now uses CreateItemSet so it doesn't double add the set anymore Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Avoid reapplying active loadout sets Only switch tree, item, skill, and config sets when the selected loadout uses a different set from the currently active one. This restores the previous loadout switching behaviour and prevents build initialisation from reprocessing stale tab state from the prior build, which caused the test suite to fail when resetting between skill tests. Co-authored-by: LocalIdentity <localidentity2@gmail.com>
* Transferred new loadout with link identifier fix from PoB2
Partially Fixes#9451
Description of the problem being solved:
Testing Needed
Before screenshot:
After screenshot:
Loadout Dropdown - New Manage option replacing New Loadout option
Loadout Management List Control
Rename Loadout
Copy Loadout
New/Copy Custom
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.