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

Various examples are incorrectly using GREEN #12225

Closed
2 tasks
rparrett opened this issue Mar 1, 2024 · 1 comment · Fixed by #12328
Closed
2 tasks

Various examples are incorrectly using GREEN #12225

rparrett opened this issue Mar 1, 2024 · 1 comment · Fixed by #12328
Labels
A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@rparrett
Copy link
Contributor

rparrett commented Mar 1, 2024

Bevy version

main, after #12163

What went wrong

So we've got a few different greens now.

pub const GREEN: LegacyColor = LegacyColor::rgb(0.0, 1.0, 0.0);
// palettes::basic
pub const GREEN: Srgba = Srgba::new(0.0, 0.5, 0.0, 1.0);
// palettes::css
// (but `basic` is re-exported in `css`, which possibly overrides this?)
// note: I think that this difference is probably large enough to cause very slight "real" differences
// depending on what sort of conversions or rounding it goes through.
pub const GREEN: Srgba = Srgba::new(0.0, 0.502, 0.0, 1.0);
// LinearRgba
pub const GREEN: Self = Self {
    red: 0.0,
    green: 1.0,
    blue: 0.0,
    alpha: 1.0,
};

And this is causing some problems in various examples that previously used the full-green LegacyColor::GREEN.

The most obvious to me is bounding_2d example where the ray intersection is drawn in green, which is much harder to see now.

We should:

  • look at our usages of GREEN throughout examples and see if those should be LIME.
  • remove GREEN from the css palette.

Note: LIME is "full green" according to the css specs, and LIMEGREEN is a separate color that is inbetween LIME and GREEN.

Examples don't need to be identical to how they looked previous to the big migration, but if a very bright green was intended and now it is very dark we may want to fix that.

https://thebevyflock.github.io/bevy-example-runner/ will be handy for tracking these down.

@rparrett rparrett added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 1, 2024
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 1, 2024
@alice-i-cecile
Copy link
Member

The duplicated re-export vs definition is a bug, and should just be fixed.

And yeah, I care a lot more about functional results than consistency with a previously arbitrary color.

@rparrett rparrett changed the title Various examples are incorrectly using GREEN Various examples are incorrectly using GREEN Mar 1, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
# Objective

Addresses one of the side-notes in #12225.

Colors in the `basic` palette are inconsistent in a few ways:
- `CYAN` was named `AQUA` in the referenced spec. (an alias was added in
a later spec)
- Colors are defined with e.g. "half green" having a `g` value of `0.5`.
But any spec would have been based on 8-bit color, so `0x80 / 0xFF` or
`128 / 255` or ~`0.502`. This precision is likely meaningful when doing
color math/rounding.

## Solution

Regenerate the colors from
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=37563bedc8858033bd8b8380328c5230
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
# Objective

Fixes #12225

Prior to the `bevy_color` port, `GREEN` used to mean "full green." But
it is now a much darker color matching the css1 spec.

## Solution

Change usages of `basic::GREEN` or `css::GREEN` to `LIME` to restore the
examples to their former colors.

This also removes the duplicate definition of `GREEN` from `css`. (it
was already re-exported from `basic`)

## Note

A lot of these examples could use nicer colors. I'm not trying to do
that here.

"Dark Grey" will be tackled separately and has its own tracking issue.
spectria-limina pushed a commit to spectria-limina/bevy that referenced this issue Mar 9, 2024
…e#12323)

# Objective

Addresses one of the side-notes in bevyengine#12225.

Colors in the `basic` palette are inconsistent in a few ways:
- `CYAN` was named `AQUA` in the referenced spec. (an alias was added in
a later spec)
- Colors are defined with e.g. "half green" having a `g` value of `0.5`.
But any spec would have been based on 8-bit color, so `0x80 / 0xFF` or
`128 / 255` or ~`0.502`. This precision is likely meaningful when doing
color math/rounding.

## Solution

Regenerate the colors from
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=37563bedc8858033bd8b8380328c5230
spectria-limina pushed a commit to spectria-limina/bevy that referenced this issue Mar 9, 2024
# Objective

Fixes bevyengine#12225

Prior to the `bevy_color` port, `GREEN` used to mean "full green." But
it is now a much darker color matching the css1 spec.

## Solution

Change usages of `basic::GREEN` or `css::GREEN` to `LIME` to restore the
examples to their former colors.

This also removes the duplicate definition of `GREEN` from `css`. (it
was already re-exported from `basic`)

## Note

A lot of these examples could use nicer colors. I'm not trying to do
that here.

"Dark Grey" will be tackled separately and has its own tracking issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants