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

inventory_column: improve sorting #53373

Merged
merged 5 commits into from
Dec 13, 2021
Merged

Conversation

andrei8l
Copy link
Contributor

Summary

None

Purpose of change

Re-run of #53289

Items are sorted haphazardly in most windows based on inventory_selector (most notably in the pickup menu)
Fixes: #52947

Describe the solution

  1. Sort items at the lowest common level, i.e. sort them by their respective parents from the level below their lowest common ancestor
  2. Sort items by unprefixed name first (same as in the AIM)
  3. identical items are disambiguated by entry generation number

Describe alternatives you've considered

  • Fixing and using order_by_parent(): I'd have to understand it first
  • Sorting in multiple steps: seems too crude

Testing

  • Confirm that player inventory can still be displayed in hierarchical mode
  • Stash a lot of items in nested containers, then drop the containers on the floor, then open the pickup menu
  • Collapse and un-collapse containers in the pick up menu. New items should be placed appropriately and the order should not get messed up (including in the player inventory window when in hierarchical mode)
  • Kill a lot of identical zombies with debug then move them to the same tile, then open the pickup menu. Items should show below their respective zombie corpse parent instead of all at the end
  • Spawn and drop 50 glass jars of sauerkraut then open the pickup menu and confirm that all the sauerkraut is sorted below the aggregated jars. Repeat with arbitrarily large numbers (machine dependent).
  • Item menus that don't indent (ex: Eat menu) should be unaffected

Additional context

before

github bad

after edit: this screenshot still contains the bug with alphabetic sorting, but it should be fixed now

github good

ITEMS_WORN is now a sortable category like the rest so it looks a little different in the player inventory window.

andrei8l and others added 5 commits December 11, 2021 20:50
properly sort entries alphabetically
sort entries below their parents
send it back whence it came
and use it to completely define sort order
@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Dec 11, 2021
@andrei8l
Copy link
Contributor Author

@kevingranade I had tried the approach in 508f34d before without success and didn't realize that fef372f actually made it work. I had to open a new PR because I force pushed to the branch; sorry about that.

@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Dec 11, 2021
@Maleclypse Maleclypse added the Controls / Input Keyboard, mouse, keybindings, input UI, etc. label Dec 12, 2021
@kevingranade kevingranade merged commit 404b140 into CleverRaven:master Dec 13, 2021
@andrei8l
Copy link
Contributor Author

Thanks for merging!

@andrei8l andrei8l deleted the inv_sorting branch December 13, 2021 06:52
@wapcaplet wapcaplet added <Bugfix> This is a fix for a bug (or closes open issue) Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones labels Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) Controls / Input Keyboard, mouse, keybindings, input UI, etc. Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pickup menu items are not displayed within their container
4 participants