Add support for Renaming, Copying, and Deleting Loadouts#9528
Open
Peechey wants to merge 10 commits intoPathOfBuildingCommunity:devfrom
Open
Add support for Renaming, Copying, and Deleting Loadouts#9528Peechey wants to merge 10 commits intoPathOfBuildingCommunity:devfrom
Peechey wants to merge 10 commits intoPathOfBuildingCommunity:devfrom
Conversation
… for clean up and other comments
clean up old comments, remove unused variables refactor loops so we only do it once and set id, index as needed
612aa6e to
0e38589
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the problem being solved:
Initial impl for Renaming, Copying, and Deleting Loadouts. Also allows creating a New Loadout from current active sets. There are a lot of changes from over a year of off and on dev so I don't remember everything and I'm gonna comb through it over the next days. Getting eyes on for optimizations, improvements, testing, and other questions about dumb things I'm probably doing.
There is commented code I need to remove, unused variables likely, and more comments to add.Concerns/Questions for Maintainers
Build.lua 324 ish. I'm reusing the copy and kind of delete functions from the SetListControls of each respective type. So I'm basically making headless ListControls every single time we click anything in the Loadouts dropdown. I don't know if that's okay or a performance tank or overall bad but it's there. Maybe I can update it to only create those for when we need them aka when it's one of the functions?This has been updated to only init the ListControls during Copy and Delete.Speaking of Build.lua the Loadout logic is starting to really grow, is there a way to move this to its own file and load it in to Build instead for clarity?
The DeleteByIndex functions I made for each ListControl is only because I want one popup confirmation for all 4 deletes and not trigger 4 separate confirmations if I were to call the OnSelDelete of each ListControl. However these are identical functions inside of the popup wrapper, is there a way to reuse OnSelDelete and bypass the confirmation?
Test Cases
For every test, I'm using an exact match Loadout and an identifier Loadout. Double the loadouts, double the fun. I also found a pob with a ton of loadouts that I'm maniacally butchering.
New Loadout
Create a new Tree other than Default, verify 2 loadouts exist, Default and newTree -> load newTree -> click New Loadout with existing sets named Loadout 1 -> Expect newTree loadout to disappear and only Default and Loadout 1 to exist as there is no newTree item, skill, config
Copy Default loadout 3 times for Loadout 1, Loadout 2, Loadout 3 -> manually change Tree to Loadout 1, Item to Loadout 2, Skill to Loadout 3, and Config to Default -> click New Loadout with existing sets renamed Loadout 4 -> expect new Loadout 4 to exist in the list and the other Loadouts unchanged
Rename Loadout
Copy Loadout
Delete Loadout
Misc
After screenshot:
Delete has a confirmation
