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

Group and sort bionics in @ screen #60228

Merged
merged 1 commit into from Aug 23, 2022
Merged

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Aug 17, 2022

Summary

Interface "Group and sort bionics on @ screen"

Purpose of change

Resolves part of #54192
Fixes #60093

Describe the solution

Use localized sort instead of random (install?) order
Group up bionics of same type and display count instead of duplicates

Describe alternatives you've considered

Testing

Install a few bionics that allow dupes (power storage, mk2, trickle chargers, electromagnetic leak)
See they're separate and all bionics are ordered in what looks like install order in @ screen
Apply patch
They now should be sorted by localized names and grouped similar to screenshot

Additional context

image

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 17, 2022
@Night-Pryanik
Copy link
Contributor

Personally I'd prefer Power Storage (16) instead of 16 Power Storage as numbers in the front visually disrupt sorting for me.

@irwiss
Copy link
Contributor Author

irwiss commented Aug 17, 2022

That's fair, I fiddled around with it but couldn't find something that looks ok, ended up trying to imitate inventory

@irwiss irwiss marked this pull request as ready for review August 17, 2022 15:42
@mqrause
Copy link
Contributor

mqrause commented Aug 17, 2022

Is it feasible to do the grouping directly in the my_bionics vector instead of just grouping it up for display?

@irwiss
Copy link
Contributor Author

irwiss commented Aug 17, 2022

Don't think so, the UI code for @ display only cares about bionic type names, description and amounts.

Actual my_bionics is vector of bionic struct which contains more stuff; fuel state, autostart thresholds etc which are per-bionic rather than per-category

@mqrause
Copy link
Contributor

mqrause commented Aug 17, 2022

Yeah, the bionics would have to be stored in the bionic_grouping struct (or a different implementation), too. It'd definitely be a much more involved change. I don't know much about the bionic code, hence the question about feasibility. But there's probably not enough to gain by the amount of work it probably means.

@Fris0uman Fris0uman merged commit e050379 into CleverRaven:master Aug 23, 2022
@irwiss irwiss deleted the group-bionics branch August 23, 2022 12:33
Hirmuolio pushed a commit to Hirmuolio/Cataclysm-DDA that referenced this pull request Aug 27, 2022
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 BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bionics with multiple copies installed should stack, rather than be listed individually
4 participants