Issue 3353 - Improved arsenal's default sort behaviour - #3951
Open
DairyProductsz wants to merge 2 commits into
Open
Issue 3353 - Improved arsenal's default sort behaviour#3951DairyProductsz wants to merge 2 commits into
DairyProductsz wants to merge 2 commits into
Conversation
Currently when opening the arsenal, items may appear in an unsorted manner by default (e.g. as noted in issue official-antistasi-community#3353, headgear appears loaded initially in a random order). Clicking on the "default" sort option also doesn't do any sorting, and items will stay listed in the last way they were sorted in. This PR makes the "default" sort option alphabetical, and items will now be displayed alphabetically when the arsenal is initially loaded as well as when "default" is selected. Also, as specified in official-antistasi-community#3353, the Antistasi berets will appear first in the list if you are viewing the headgear section with the "default" sort.
Author
|
Made a mistake with this actually. Apparently when you open the arsenal by default, weapons are listed in the order you've obtained them in, and clicking the "default" sorting option functions correctly with them. The issue only seems to happen specifically with misc. items like headgear where everything was already unlocked, so some further changes are needed. |
- Default sort now acts similarly to lbSort (current default sort method), with the exception of prioritizing the Antistasi beret items - Restored original code for alphabetical sort - Fixed bug with "amount" based sort; lbSortByValue call was inside of the for loop
Author
|
Did some more testing with looted items and made some changes, I think the sorting behaviour should be correct now. I updated the PR description with details. |
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.
What type of PR is this.
What have you changed and why?
1. Default sort
Currently when opening the arsenal, pre-unlocked rebel items may appear in an unsorted manner by default (e.g. as noted in issue #3353, headgear appears loaded initially in a random order), although looted items will appear in the order they were obtained. However if you select some different sorting options and then click again on the "default" sort option, the
lbSortcall will sort items by the text of the_ctrlListitems. This sorting works fine actually, since it effectively sorts items first by amount and then alphabetically, though it is also inconsistent behaviour to what you see when you initially open the arsenal.This PR makes the default sort acts as a slightly modified lbSort. As specified in #3353, the Antistasi berets will appear first in the list if you are viewing the headgear section with the "default" sort, while everything else will sort itself by its text (i.e. by amount) as occurs with regular lbSort. The modified lbSort will also be called when initially loading the arsenal as well for consistency.
A small caveat to this solution is that when opening the arsenal, items won't appear in the order they were obtained/looted, but I think sorting by amount + name is still a good default sorting behaviour. If the original behaviour is important to maintain though, I'll make further changes.
2. Fixed sort by amount
In the case for sorting items by amount,
lbSortByValue _ctrlList;was called inside the for loop, which occasionally caused items to be listed in the wrong order. I moved it outside the for loop, and everything seems fixed now.Please specify which Issue this PR Resolves.
closes #3353
Please verify the following and ensure all checks are completed.
Is further testing or are further changes required?
How can the changes be tested?
Steps: Open the arsenal and verify the items are sorted properly. Check the headgear in the "default" sort, and the Antistasi berets should be the first items in that list.
Notes:
N/A.