Skip to content

Add missing HTML colors to Color struct.#4978

Closed
BreadcrumbIsTaken wants to merge 2 commits intobevyengine:mainfrom
BreadcrumbIsTaken:main
Closed

Add missing HTML colors to Color struct.#4978
BreadcrumbIsTaken wants to merge 2 commits intobevyengine:mainfrom
BreadcrumbIsTaken:main

Conversation

@BreadcrumbIsTaken
Copy link
Copy Markdown

@BreadcrumbIsTaken BreadcrumbIsTaken commented Jun 9, 2022

As well as updating the PINK constant to match the HTML color. It was originally set as the "Deep Pink" HTML color, but now matches the "Pink" HTML color.

Objective

  • To add more colors from HTML following the practice of other game engines/frameworks.

Solution

  • Add missing HTML colors.
  • Fix misrepresented color (PINK).

As well as updating the PINK constant to match the HTML color. It was originally set as the "Deep Pink" HTML color, but now matches the "Pink" HTML color.
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 9, 2022
Copy link
Copy Markdown
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.

Can you add a link to the HTML colors as a comment above this impl block? Otherwise this changes looks good to me :)

@DJMcNab
Copy link
Copy Markdown
Member

DJMcNab commented Jun 9, 2022

We need to follow the license for the colours list too. As best as I can find, that would be https://www.w3.org/Consortium/Legal/2015/copyright-software-and-document
But if we can clarity from anyone else that would be good.

@alice-i-cecile
Copy link
Copy Markdown
Member

My opinion is that this list is in fact copyrightable because it is a deliberate collection, so I'd like a license note as part of the comment mentioned above.

@BreadcrumbIsTaken
Copy link
Copy Markdown
Author

Just to double check, but do you want a link to the list of HTML colors that I used and the link to the license found by DJMcNab?

@alice-i-cecile
Copy link
Copy Markdown
Member

Yes, exactly :)

@BreadcrumbIsTaken
Copy link
Copy Markdown
Author

Got it! ;)
Making the changes right now.

@DJMcNab DJMcNab added the X-Needs-SME This type of work requires an SME to approve it. label Jun 9, 2022
@IceSentry
Copy link
Copy Markdown
Contributor

I'm not convinced this is actually needed, having primary, secondary and tertiary colors makes sense, but something like CHOCOLATE or CHARTREUSE is just too specific. If people want specific colors they can easily create their own constants to build a palette. A lot of those color alias exist for legacy rerasons in html but aren't really standardized in any way.

@alice-i-cecile
Copy link
Copy Markdown
Member

Hmm. Maybe we should just yank this collection entirely and put in a sensible modern color palette instead?

@BreadcrumbIsTaken
Copy link
Copy Markdown
Author

BreadcrumbIsTaken commented Jun 9, 2022

That sounds like a good idea, but the reason I made this PR was because I noticed that some of the HTML colors were already added to Bevy while some where not. However specific they may be I thought it would be a good idea to maybe follow the practices of other game engines/frameworks like Godot, pygame, and MonoGame. All have a complete set of HTML colors and pygame even has a bit more. A possible idea is that we could include a complete set of HTML colors as well as an updated more modern set of colors as well.
But having modern color palettes instead will still be a good and sensible option.

@IceSentry
Copy link
Copy Markdown
Contributor

Personally, if we do end up with all the html colors I would much prefer if they were in their own html_palette or css_palette submodule and not just directly on Color

@BreadcrumbIsTaken
Copy link
Copy Markdown
Author

Personally, if we do end up with all the html colors I would much prefer if they were in their own html_palette or css_palette submodule and not just directly on Color

Sounds like a good idea. I could try to add it to this PR as it seems that it would work out.

Copy link
Copy Markdown
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

I know this is a pain, but this is what W3C wants.

},
}

// List of HTML colors sourced from https://www.w3schools.com/tags/ref_colornames.asp.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the wrong link - please re-derive based on https://www.w3.org/TR/css-color-4/#named-colors. In theory, I think we also need to re-derive the list from that source. I'm not sure how that works when the list would be the same here, but when moving them between modules, please re-derive them. Thanks

(The w3schools page is all rights reserved, so we can't use it.)

}

// List of HTML colors sourced from https://www.w3schools.com/tags/ref_colornames.asp.
// Extended color keywords license: https://www.w3.org/Consortium/Legal/2015/copyright-software-and-document.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to actually follow this license, not just link it. Also, I'm afraid I linked the wrong license - it's actually this one https://www.w3.org/Consortium/Legal/2015/doc-license (linked from https://www.w3.org/TR/css-color-4/#named-colors:
image
)
I think we need:

Copyright © 2022 W3C® (MIT, ERCIM, Keio, Beihang). W3C liability, trademark and permissive document license rules apply.

However, this also adds new restrictions to the final game created (so long as any of these colour constants get compiled in?), so it might be better to move it to a seperate crate entirely.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seperate crate would need this license item https://spdx.org/licenses/W3C-20150513.html

@BreadcrumbIsTaken
Copy link
Copy Markdown
Author

@DJMcNab I think you are right about making this a different crate. Although the HTML colors may not be able to be in Bevy by default but the idea about a modern palette could still be a possible solution.

@IceSentry
Copy link
Copy Markdown
Contributor

Closing since we now have a css palette in bevy_color #12067

@IceSentry IceSentry closed this May 2, 2024
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 targeted quality-of-life change that makes Bevy easier to use X-Needs-SME This type of work requires an SME to approve it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants