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

Split up roads.mss, refactor road colour script #2220

Merged
merged 6 commits into from Jul 13, 2016

Conversation

jdhoek
Copy link
Contributor

@jdhoek jdhoek commented Jul 10, 2016

The generated colour values now reside in road-colors-generated.mss, and
have been removed from roads.mms. road-colors-generated.mms is generated
by the colour script, and should not be edited manually. To regenerate
it, edit road-colors.yaml and run:

./scripts/generate_road_colours.py > road-colors-generated.mss

The original output of the colour script provided some info about the
amount of loss caused by converting from CIE lch to RGB. To get this
output, call the script with the -v flag (for verbose).

@jdhoek
Copy link
Contributor Author

jdhoek commented Jul 10, 2016

This replaces the refactoring part of #2219.

Related to #2085.

@jdhoek jdhoek force-pushed the feature/refactor-colour-script branch 2 times, most recently from 565f9df to dadb8cd Compare July 10, 2016 19:12
@jdhoek jdhoek force-pushed the feature/refactor-colour-script branch 2 times, most recently from 847315e to 1bd1ff0 Compare July 10, 2016 20:13
@pnorman
Copy link
Collaborator

pnorman commented Jul 10, 2016

👍 in principle, but I haven't done a code review yet

@pnorman
Copy link
Collaborator

pnorman commented Jul 10, 2016

I opened jdhoek#1 with some changes and support for road shields

Can tertiary not be special-cased in generate_shields.py and generated from generate_road_colours.py? If there's no way to easily do so it's okay as it is, but then we should add a comment to generate_road_colours.py about where the rgb hex values for it come from

@jdhoek
Copy link
Contributor Author

jdhoek commented Jul 11, 2016

@pnorman Great, I'll have a look.

@HolgerJeromin
Copy link
Contributor

We could check the status of road-colors.yaml (that it generates the same file as the checked in version) with Travis. But this could be done later.

@jdhoek
Copy link
Contributor Author

jdhoek commented Jul 11, 2016

With the help of @pnorman generate_shields.py now uses the settings from road-colors.yaml as well. For both generating files: their output is unchanged.

jdhoek and others added 6 commits July 11, 2016 19:36
The generated colour values now reside in road-colors-generated.mss, and
have been removed from roads.mms. road-colors-generated.mms is generated
by the colour script, and should not be edited manually. To regenerate
it, edit road-colors.yaml and run:

```
./scripts/generate_road_colours.py > road-colors-generated.mss
```

The original output of the colour script provided some info about the
amount of loss caused by converting from CIE lch to RGB. To get this
output, call the script with the `-v` flag (for verbose).
Refactor generate_shields.py to use road-colors.yaml as well. It now
uses methods from generate_road_colours.py to do this.
@jdhoek jdhoek force-pushed the feature/refactor-colour-script branch from eb2063a to d13cb88 Compare July 11, 2016 17:37
@pnorman
Copy link
Collaborator

pnorman commented Jul 13, 2016

generate_shields.py now uses the settings from road-colors.yaml as well.

Thanks. I thought about doing this but didn't want to get into that script as well.

@jdhoek
Copy link
Contributor Author

jdhoek commented Jul 13, 2016

Turned out to be a minor change in that script, so it seemed beneficial. I did alter the spacing to align with Python standards and generate_road_colours.py, but only a handful of lines were actually altered.

@pnorman pnorman merged commit 60ed128 into gravitystorm:master Jul 13, 2016
@matthijsmelissen
Copy link
Collaborator

Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants