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

[RAINBOW EFFECT] Added methods to get HSL components from Color #9201

Merged
merged 4 commits into from
Aug 21, 2023
Merged

Conversation

IDEDARY
Copy link
Contributor

@IDEDARY IDEDARY commented Jul 18, 2023

Changes

Added methods to Color enum to retrieve Hue, Saturation and Lightness values.

Why?

As you probably know, to create a color that iterates over the color spectrum (rainbow effect that can be seen on LED keyboards, PC components, etc..), you need to mix the color from Hue, Saturation and Luminosity. Bevy already supports multiple color formats, but provides only 4 methods of retrieving components for RGBA. Nothing like ".get_hue()", so I implemented them with all their variations that RGBA has.

Now we can do true rainbow color blending (Example is a button hover effect): Discord Showcase, Video download

image

I needed to expose the components of HSL color format for rainbow tweening and color blending. RGBA is not enough. Blending through RGBA goes through white.
@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Consistent with existing code and useful.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use labels Jul 18, 2023
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

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

LGTM

@nicopap nicopap added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 19, 2023
@nicopap nicopap added this to the 0.12 milestone Jul 19, 2023
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
@mockersf mockersf removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 19, 2023
I left sRGB mentions in comments for color creation, just removed the ones we talked about.
@IDEDARY IDEDARY requested a review from mockersf July 19, 2023 13:43
@IDEDARY
Copy link
Contributor Author

IDEDARY commented Jul 19, 2023

I added another commit removing the comment mentions, now we just need to wait for @mockersf to finish the change request

crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@Selene-Amanita Selene-Amanita added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 15, 2023
@Selene-Amanita
Copy link
Member

Selene-Amanita commented Aug 15, 2023

(note that it segfaulted at the example and that's a known problem with CI right now, but you also have CI validation problems that need to be fixed)

/// Color::Hsla
/// to
/// [`Color::Hsla`]

@IDEDARY
Copy link
Contributor Author

IDEDARY commented Aug 15, 2023

(note that it segfaulted at the example and that's a known problem with CI right now, but you also have CI validation problems that need to be fixed)

/// Color::Hsla
/// to
/// [`Color::Hsla`]

Yeah I saw the discussion on Discord about it. I also added the interactive links to my comments as you suggested.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@IDEDARY
Copy link
Contributor Author

IDEDARY commented Aug 15, 2023

I also kind of noticed that this fixes the same issue as #7759, so I guess cart can decide which PR he likes more. The other PR has more specific naming than mine as I tried to be consistent with existing code.

@Selene-Amanita
Copy link
Member

I approved both because I'm fine with both personally (I know it doesn't help much ^^')

@alice-i-cecile
Copy link
Member

This is fine as is: we can clean things up later if people care enough.

Merged via the queue into bevyengine:main with commit af0323a Aug 21, 2023
19 of 20 checks passed
@cart cart mentioned this pull request Oct 13, 2023
43 tasks
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-Usability A simple quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants