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 improvements to the contributors example #12217

Merged

Conversation

rparrett
Copy link
Contributor

Objective / Solution

And a few other improvements:

  • Make contributor colors persistent between runs
  • Move text to top left where the birbs can't reach and add padding
  • Remove Contributor: text. It's obvious what is being shown from context, and then we can remove has_triggered.
  • Show the number of commits authored
  • Some system names were postfixed with _system and some weren't. Removed _system for consistency.
  • Clean up collision code slightly with a bounding volume
  • Someone accidentally typed "bird" instead of "birb" in one comment.
  • Other misc. cleanup

Before

image

After

image

- [X] Make contributor colors persistent between runs
- [X] Move text to top left and add padding
- [X] Remove `Contributor:` text. It's obvious what is being shown from context, and we can simplify `SelectionState`.
- [X] Show the number of commits authored
- [X] Use `Hsla` for the color constants
- [ ] ? Clean up collision code with bounding volumes
@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples C-Code-Quality A section of code that is hard to understand or change labels Feb 29, 2024

Ok(contributors)
}

/// Give each unique contributor name a particular hue that is stable between runs.
fn name_to_hue(s: &str) -> f32 {
Copy link
Member

Choose a reason for hiding this comment

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

Can we combine this with #12173 to get a nicely dispersed collection of hues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I played with this for funsies, but I feel like the distribution of hues from the hash alone is probably good enough.

image

@alice-i-cecile
Copy link
Member

I like the commit count: that's really neat!

Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

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

Really nice changes!

const SATURATION_SELECTED: f32 = 0.9;
const LIGHTNESS_SELECTED: f32 = 0.7;
const ALPHA: f32 = 0.92;
const SELECTED: Hsla = Hsla::hsl(0.0, 0.9, 0.7);
Copy link
Contributor

Choose a reason for hiding this comment

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

Much nicer! Only suggestion is Oklch might look nicer since it has better uniformity

Copy link
Contributor Author

@rparrett rparrett Mar 1, 2024

Choose a reason for hiding this comment

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

I'm honestly not sure how to use Oklch. Maybe I read up a bit more. I naively converted these values to Oklch and kept the rest of the code the same and some of them look okay but most look dark and saturated.

let delta = time.delta_seconds();

for mut velocity in &mut velocity_query {
velocity.translation.y -= GRAVITY * delta;
}
}

/// Checks for collisions of contributor-birds.
/// Checks for collisions of contributor-birbs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfection

@rparrett rparrett 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 Mar 1, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 1, 2024
Merged via the queue into bevyengine:main with commit bcdca06 Mar 1, 2024
29 checks passed
spectria-limina pushed a commit to spectria-limina/bevy that referenced this pull request Mar 9, 2024
# Objective / Solution

- Use `Hsla` for the color constants (Fixes bevyengine#12203)

And a few other improvements:

- Make contributor colors persistent between runs
- Move text to top left where the birbs can't reach and add padding
- Remove `Contributor:` text. It's obvious what is being shown from
context, and then we can remove `has_triggered`.
- Show the number of commits authored
- Some system names were postfixed with `_system` and some weren't.
Removed `_system` for consistency.
- Clean up collision code slightly with a bounding volume
- Someone accidentally typed "bird" instead of "birb" in one comment.
- Other misc. cleanup

## Before

<img width="1280" alt="image"
src="https://github.com/bevyengine/bevy/assets/200550/9c6229d6-313a-464d-8a97-0220aa16901f">

## After

<img width="1280" alt="image"
src="https://github.com/bevyengine/bevy/assets/200550/0c00e95b-2f50-4f50-b177-def4ca405313">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples 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.

Clean up color logic in contributors.rs example
4 participants