Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFeature: Group liveries, and livery window usability enhancements. #7108
Conversation
This comment has been minimized.
This comment has been minimized.
A PR like this ought to have some PR screenshots, I say. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Thanks Andy. Didn't have time earlier to post screenshots. |
This comment has been minimized.
This comment has been minimized.
Company Colour defaults are now set like this: For ungrouped trains, RVs etc, the long established (and weird imo) settings for some types of vehicle are kept. Note however that the checkboxes are now removed, reducing UI friction: Liveries for groups do not offer settings by types of vehicle, 1CC and 2CC will be applied consistently to all vehicles in the group: Results: Group livery UI can also be opened directly from the groups window, with the correct group selected: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looks good. I tested it and works as expected. But please run script/api/generate_widget.sh. Some questions:
|
This comment has been minimized.
This comment has been minimized.
Ah yes. We really ought to make this part of the build process!
Hmm, sounds reasonable, although the title widget is already the company colour. EDIT Or not. Confused as I was testing something else and it was. EDIT And it is in master, just broken in this PR, wow.
Possibly depending on the next point.
This will change how setting a colour to 'Default' will work. Currently it means to fall back to the system of picking the colour based on the original liveries system. With hierarchy in place it would mean falling back to the parent scheme.
It's based on the extra OpenTTD GUI sprites that already exist, but there's no icon for 'choose colour'. |
PeterN
force-pushed the
PeterN:group-livery
branch
from
5d1cb6d
to
92499e0
Jan 27, 2019
This comment has been minimized.
This comment has been minimized.
nielsmh
reviewed
Jan 27, 2019
I will admit to not testing this myself, but the code looks sound, and others' testing indicates it works as intended. |
PeterN
dismissed
nielsmh’s
stale review
via
9383c79
Jan 27, 2019
PeterN
force-pushed the
PeterN:group-livery
branch
from
92499e0
to
9383c79
Jan 27, 2019
This comment has been minimized.
This comment has been minimized.
Another update as I had, again, forgotten to run generate_widgets |
This comment has been minimized.
This comment has been minimized.
Well, instead of "Default", it should be "Inherited". But it is more consistent to inherit colour from parents, the same way autoreplacements work.
I think it is better to deal with this now for keeping commits to master in a better order. |
This comment has been minimized.
This comment has been minimized.
About icons, it is my fault. I thought those icons were based on non-free graphics instead of being based on OpenGFX. Probably, some static newGRF must have had replaced the ones in OpenGFX when I checked this. |
PeterN
force-pushed the
PeterN:group-livery
branch
from
9383c79
to
dc8362a
Jan 27, 2019
This comment has been minimized.
This comment has been minimized.
@nielsmh Unless someone wants to improve them, I think the icons are fine. Current version now displays the group hierarchy so the list order is the same as the group window, but does not affect how colour is assigned. There's a small discrepancy anyway that when 'Default' is selected it shows the main company colour, but vehicles may not be that colour if the type-based liveries are used. |
PeterN
force-pushed the
PeterN:group-livery
branch
from
dc8362a
to
73cde85
Jan 28, 2019
PeterN
added some commits
Apr 16, 2018
PeterN
force-pushed the
PeterN:group-livery
branch
from
73cde85
to
b115c53
Jan 28, 2019
This comment has been minimized.
This comment has been minimized.
Group hierarchy is now followed for liveries. |
nielsmh
added
Interface
enhancement
savegame upgrade
labels
Jan 28, 2019
This comment has been minimized.
This comment has been minimized.
Tested, works for me. I thought I had found an issue where default was sometimes wrong for 2CC, but can't repro. Only saw it in a savegame with groups created in a previous version of patch, assuming it was stale data. |
This comment has been minimized.
This comment has been minimized.
It compiles and looks good. Let's ship it ;) |
planetmaker
approved these changes
Jan 31, 2019
As by the comment... let's ship it |
PeterN commentedJan 26, 2019
•
edited
This PR add liveries to vehicle groups, and makes some usability changes to the livery selection window, replacing the checkbox enable/disable with a 'default' option in the colour drop down.