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

Sort unit lists shown to the user #2288

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

lmoureaux
Copy link
Contributor

On top of #2287 - check the second commit.

This patch makes the user interface look more consistent by sorting
units in two contexts:

  • In the city dialog
  • In the unit picker

The sorting logic is an improved version of code formerly used to sort
units in the city dialog (but that had been lost for some reasion). It
tries to produce a sort order that looks natural and is stable in time.
To do so:

  • Loaded units are always sorted right after their transport, so one can
    tell in which transport they are loaded;
  • The best defensive units go to the beginning of the list. These tend
    to stay in cities and not move around too much;
  • Tie breaker by unit type and nationality ensure similar units are
    grouped together;
  • Finally, the unit id is used as a last resort.

I tested this logic in a few contexts and it seems to achieve its goal.
With respect to the previous sorting function, I removed an order based
on unit activities (fortified, fortifying, sentried) because it felt
wrong to have a unit move in the list just because I changed its
activity.

Closes #1836.
See #1838.

This patch makes the user interface look more consistent by sorting
units in two contexts:

* In the city dialog
* In the unit picker

The sorting logic is an improved version of code formerly used to sort
units in the city dialog (but that had been lost for some reasion). It
tries to produce a sort order that looks natural and is stable in time.
To do so:

* Loaded units are always sorted right after their transport, so one can
  tell in which transport they are loaded;
* The best defensive units go to the beginning of the list. These tend
  to stay in cities and not move around too much;
* Tie breaker by unit type and nationality ensure similar units are
  grouped together;
* Finally, the unit id is used as a last resort.

I tested this logic in a few contexts and it seems to achieve its goal.
With respect to the previous sorting function, I removed an order based
on unit activities (fortified, fortifying, sentried) because it felt
wrong to have a unit move in the list just because I changed its
activity.

Closes longturn#1836.
See longturn#1838.
@lmoureaux lmoureaux force-pushed the feature/sort-units branch from 46ebd5e to 83bde24 Compare May 21, 2024 23:07
Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems to work as described. Wondering if it makes sense to expand the units in city to two fully visible rows when applicable and then give a scroll bar for rows 3+

image

@lmoureaux
Copy link
Contributor Author

The order you show is not what I intended, the planes should come right after the carrier and the defending rifles should be first. This is strange...

I agree that more rows should be shown if possible, but right now I'm not sure how to do that (it's not the same widget as in the unit thing).

@jwrober
Copy link
Collaborator

jwrober commented May 21, 2024

Ok. Loaded units "before" vs "after" is a thing. As long as we are consistent either is ok with me. Maybe sorting still needs some work. I'm also ok with tackling the city dialog scaling to two rows later.

@jwrober
Copy link
Collaborator

jwrober commented May 25, 2024

Everything looks good except for the situation where a ship with loaded units is present in the city. The dialog shows the ship + loaded units first and then best defender.

@lmoureaux
Copy link
Contributor Author

Ok will check, thanks

@lmoureaux
Copy link
Contributor Author

Everything looks good except for the situation where a ship with loaded units is present in the city. The dialog shows the ship + loaded units first and then best defender.

I can't reproduce this behavior

@lmoureaux lmoureaux merged commit 02a20ae into longturn:master Jun 13, 2024
22 checks passed
@lmoureaux lmoureaux deleted the feature/sort-units branch June 13, 2024 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the UI for transport ships with loaded units in city view
2 participants