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

Jump to bottom

Sort cargo by class / name in CompanyStationsWindow #5311

Closed
DorpsGek opened this issue Sep 29, 2012 · 6 comments
Closed

Sort cargo by class / name in CompanyStationsWindow #5311

DorpsGek opened this issue Sep 29, 2012 · 6 comments
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

sbr opened the ticket and wrote:

The first patch use _sorted_cargo_specs to iterate over cargoes.
The second patch remove cs->IsValid() calls as _sorted_cargo_specs contains only valid cargoes (except for in-game change of industry NewGRF). It also remove the empty widgets that were created for invalid cargoes.

The third patch write cargo abbrev in white if the cargo rating colour is dark. It isn't related to cargo sorting and isn't needed for standard cargoes. I found it useful for FIRS that use black as Coal rating colour.

Please find attached these patches against r24560.

Attachments

Reported version: trunk
Operating system: All

This issue was imported from FlySpray: https://bugs.openttd.org/task/5311
@DorpsGek
Copy link
Member Author

Terkhen wrote:

00 and 01 have been committed in r24615. Thanks for the patches :)

With regard to 10, using numerical values instead of constants for colours looks hacky to me. IIRC there are constants already defined somewhere. I also think that the line which uses the colour constants should have a comment describing what it is doing and why, otherwise it may not be clear what it is trying to do.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311#comment11606

@DorpsGek
Copy link
Member Author

frosch wrote:

Maybe it should look up the colour in the palette and decide dark/light depending on the RGB values?

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311#comment11607

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 7, 2012

sbr wrote:

The brightness of the cargo rating colour is now calculated using the formula found at http://alienryderflex.com/hsp.html.
I decided to put the border between dark and bright at 70. You could see the result of different settings in the attached picture.

Please find attached a patch against r24666.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311#comment11665

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 7, 2012

frosch wrote:

Thanks for looking up that formula!

I've adjusted the threshold value to 128/255, which fits the w3c suggestion of having a contrast difference of more than 125/255.
The brightness of 70/255 seemed quite dark to me.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311#comment11668

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 7, 2012

frosch closed the ticket.

Reason for closing: Implemented

in r24668

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311

@DorpsGek DorpsGek closed this as completed Nov 7, 2012
@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 8, 2012

sbr wrote:

I selected a low value for brightness to limit the difference to the only abbreviations that were hardly/un-readable.
The middle value of 128 effectively give a more balanced result.

By the way, your implementation is more elegant than mine.

This comment was imported from FlySpray: https://bugs.openttd.org/task/5311#comment11669

@DorpsGek DorpsGek added component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek