Add named colors to GDScript/Visual Script/core. #7093

bojidar-bg · 2016-11-10T21:07:53Z

~~Color names and values shamelessly stolen from @SuperUserNameMan...~~
Now from https://en.wikipedia.org/wiki/X11_color_names

Closes #6195!

bojidar-bg · 2016-11-10T21:08:17Z

core/color_names.inc

@@ -0,0 +1,1230 @@
+// Names from https://en.wikipedia.org/wiki/List_of_colors (through https://raw.githubusercontent.com/SuperUserNameMan/color_to_name/616a7cddafefda91478b7bc26167de97fb5badb1/godot_version.gd), slightly edited and normalized
+static Map<String, Color> _named_colors;
+static void _populate_named_colors() {


Dragons be here.

volzhs · 2016-11-11T06:26:46Z

core/color_names.inc

+	_named_colors.insert("pastelviolet", Color(0.80, 0.60, 0.79 ));
+	_named_colors.insert("pastelyellow", Color(0.99, 0.99, 0.59 ));
+	_named_colors.insert("patriarch", Color(0.50, 0.00, 0.50 ));
+	_named_colors.insert("payne'sgrey", Color(0.33, 0.41, 0.47 ));


is this OK using ' for color name?

bla... seems like my grepping/sedding skills failed me!

bojidar-bg · 2016-11-11T09:22:48Z

core/color_names.inc

+	_named_colors.insert("cinereous", Color(0.60, 0.51, 0.48 ));
+	_named_colors.insert("cinnabar", Color(0.89, 0.26, 0.20 ));
+	_named_colors.insert("cinnamonsatin", Color(0.80, 0.38, 0.49 ));
+	_named_colors.insert("cinnamon[citationneeded]", Color(0.82, 0.41, 0.12 ));


@volzhs That's a better example though.

pkowal1982 · 2016-11-12T22:16:43Z

core/color.cpp

@@ -327,7 +329,24 @@ bool Color::html_is_valid(const String& p_color) {

 }

-
+Color Color::named(const String &p_name) {
+	if(_named_colors.empty()) _populate_named_colors(); // from color_names.inc


Any chance to use space after if? I hope in 3.0 there will be some codestyle enforced. In other files you're using this space after if...

My bad, probably missed adding it.

pkowal1982 · 2016-11-12T22:17:12Z

core/color.cpp

+	name = name.to_lower();
+	const Map<String, Color>::Element* color = _named_colors.find(name);
+	if(color) {
+		return color->value();


if (color)...

pkowal1982 · 2016-11-12T22:50:45Z

core/color_names.inc

+static Map<String, Color> _named_colors;
+static void _populate_named_colors() {
+	if(!_named_colors.empty()) return;
+	_named_colors.insert("absolutezero", Color(0.00, 0.28, 0.73 ));


I think almost all of these colors should be thrown away and replaced by HTML ones. I found info that all modern browsers support about 140 common colors. I don't think that "awsome", "absolutezero" (no, that's not 0,0,0) or "maximumgreen" with green set to 55 percent is good choice. So maybe use these 140 standard ones instead of 1200+?

I also agree that only the HTML colors should be kept. The intention is to make it more user-friendly. Using ColorN('fuzzywuzzy') won't help with that.

Yeah, ... would fix it some time later

pkowal1982 · 2016-11-12T22:54:24Z

modules/visual_script/visual_script_builtin_funcs.cpp

@@ -64,6 +64,7 @@ const char* VisualScriptBuiltinFunc::func_name[VisualScriptBuiltinFunc::FUNC_MAX
 	"str2var",
 	"var2bytes",
 	"bytes2var",
+	"color_named",


I don't like leaving comma after the last element.

There was a comma on the previous last element, so I followed the style. I'm not changing it since "you don't like it", sorry (but, feel free to fix it everywhere when refactoring starts).

I think it's good style to keep a comma after the last element. Then when you add an element, you do a 0,+1 change and not a -1,+2.

I think it's a bad habit, as long as it compiles it can be left as is, but imagine you also add commas after last function param just because it's possible that you'll add an argument in future...
JSON forbids such syntax and I think it's good.
Commas should be used to separate elements and there is no element to be separated at the end.
Code should be as readible as possible and this -1, +2 is not a problem as review of such a change is trivial.
I should not use so many shoulds in my post. :)

@pkowal1982 Or maybe, we should use the elm-lang approach?

something = { "1" , "2" , "3" };

That way we both have no trailing commas, and we don't have a -1+2 change..

Just leave this comma as it is and shorten the list of colors. :)

pkowal1982 · 2016-11-12T22:54:32Z

modules/visual_script/visual_script_builtin_funcs.cpp

+
+			Color color = Color::named(*p_inputs[0]);
+			color.a=*p_inputs[1];
+


Sometimes you use spaces around equity sign and sometimes you do not. :)

Well, @akien-mga would fix them anyway 😛

Anyway, that's done like so everywhere in the code -- spaces around declarations of new variables only, so decided to follow that one for the case... 😄

akien-mga · 2016-11-21T10:51:44Z

I'd also be for reducing the list of colors to a smaller list of a hundred or so. 1200+ colors with names noone understands don't bring us any closer to being user friendly - best let people check a color directory on the Internet and use the HTML codes if they need a very specific color.

SuperUserNameMan · 2016-11-26T15:02:58Z

@bojidar-bg : I also think that this list of colors names you picked from my experimental test project is really too long to be included as is into Godot.

Also, I'm wondering if such addon could not just be implemented as a simple asset script instead of being made default to Godot core. (edit : though, as you mentioned the Visual scripting thing, maybe it could not be implemented as an asset anyway ........)

bojidar-bg · 2016-11-26T15:12:51Z

@SuperUserNameMan I see no problem for it to be in core...
About the length of the color list, I'm going to revise this PR one day...... (.....)

Names and values taken from https://en.wikipedia.org/wiki/X11_color_names

bojidar-bg · 2016-12-17T09:34:09Z

Ok, @akien-mga I revised the list (using colors from https://en.wikipedia.org/wiki/X11_color_names). 😄

aaronfranke · 2018-07-08T08:02:04Z

Could a similar strategy, using .insert(), be used for creating default Vectors? #7317

bojidar-bg · 2018-07-09T09:00:36Z

Not really, since it is currently implemented as another Color constructor + GDScript/VSScript function.

Vivraan · 2020-08-03T17:59:09Z

Please make these follow UPPERCASE_NAMING.

aaronfranke · 2020-08-03T19:15:13Z

@SHIVAMMUKHERJEE Thanks for the reminder, I plan to make a PR proposing replacing Color.red with Colors.RED, but I haven't gotten around to doing this yet. I just added this to my TODO list.

Vivraan · 2020-08-04T04:51:04Z

How hard could it be? I'll put this on TODO as well.

Vivraan · 2020-08-04T11:55:53Z

@aaronfranke some colours names are incorrect, such as "cornflower" instead of "cornflowerblue" and a lot of C# constants aren't named according to the Wikipedia names. I've fixed these in one commit.

How have the constants been placed under Colors? I simply can't find anywhere the Godot constants have been derived from.

Vivraan · 2020-08-04T12:02:59Z

I've made a PR for some incorrectly named color constants on both C++ and C#, but I can't wrap my head around how the color names get added to the GDScript Color description.

#41019

bojidar-bg · 2020-08-04T13:48:25Z

Please make these follow UPPERCASE_NAMING.

Necro-bumping a 2 year old PR, when an issue (or a proposal) can explain the problem better is generally not considered a good practice.

This aside, it would be better if such a comment is directed at #14704 (comment), not here, as this PR did not add Color.red, just the older ColorN("red").

bojidar-bg added feature proposal topic:core topic:gdscript topic:visualscript labels Nov 10, 2016

bojidar-bg commented Nov 10, 2016

View reviewed changes

volzhs reviewed Nov 11, 2016

View reviewed changes

bojidar-bg commented Nov 11, 2016

View reviewed changes

bojidar-bg force-pushed the named-colors branch from 7945b73 to 4bab31a Compare November 11, 2016 09:24

pkowal1982 reviewed Nov 12, 2016

View reviewed changes

Add named colors to GDScript/Visual Script/core.

23381a5

Names and values taken from https://en.wikipedia.org/wiki/X11_color_names

bojidar-bg force-pushed the named-colors branch from 4bab31a to 23381a5 Compare December 17, 2016 09:32

akien-mga approved these changes Jan 2, 2017

View reviewed changes

karroffel approved these changes Jan 10, 2017

View reviewed changes

akien-mga merged commit 57166cd into godotengine:master Jan 11, 2017

bojidar-bg deleted the named-colors branch March 30, 2017 10:39

aaronfranke added this to the 3.0 milestone Jan 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add named colors to GDScript/Visual Script/core. #7093

Add named colors to GDScript/Visual Script/core. #7093

bojidar-bg commented Nov 10, 2016 •

edited

Loading

bojidar-bg Nov 10, 2016

volzhs Nov 11, 2016

bojidar-bg Nov 11, 2016

bojidar-bg Nov 11, 2016

pkowal1982 Nov 12, 2016

bojidar-bg Nov 22, 2016

pkowal1982 Nov 12, 2016

pkowal1982 Nov 12, 2016

vnen Nov 13, 2016

bojidar-bg Nov 22, 2016

pkowal1982 Nov 12, 2016

bojidar-bg Nov 22, 2016

akien-mga Nov 22, 2016 •

edited

Loading

pkowal1982 Nov 22, 2016

bojidar-bg Nov 22, 2016

pkowal1982 Nov 22, 2016

pkowal1982 Nov 12, 2016

bojidar-bg Nov 13, 2016

akien-mga commented Nov 21, 2016

SuperUserNameMan commented Nov 26, 2016 •

edited

Loading

bojidar-bg commented Nov 26, 2016

bojidar-bg commented Dec 17, 2016

aaronfranke commented Jul 8, 2018

bojidar-bg commented Jul 9, 2018 •

edited

Loading

Vivraan commented Aug 3, 2020

aaronfranke commented Aug 3, 2020

Vivraan commented Aug 4, 2020

Vivraan commented Aug 4, 2020

Vivraan commented Aug 4, 2020

bojidar-bg commented Aug 4, 2020


		Color color = Color::named(*p_inputs[0]);
		color.a=*p_inputs[1];

Add named colors to GDScript/Visual Script/core. #7093

Add named colors to GDScript/Visual Script/core. #7093

Conversation

bojidar-bg commented Nov 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga Nov 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Nov 21, 2016

SuperUserNameMan commented Nov 26, 2016 • edited Loading

bojidar-bg commented Nov 26, 2016

bojidar-bg commented Dec 17, 2016

aaronfranke commented Jul 8, 2018

bojidar-bg commented Jul 9, 2018 • edited Loading

Vivraan commented Aug 3, 2020

aaronfranke commented Aug 3, 2020

Vivraan commented Aug 4, 2020

Vivraan commented Aug 4, 2020

Vivraan commented Aug 4, 2020

bojidar-bg commented Aug 4, 2020

bojidar-bg commented Nov 10, 2016 •

edited

Loading

akien-mga Nov 22, 2016 •

edited

Loading

SuperUserNameMan commented Nov 26, 2016 •

edited

Loading

bojidar-bg commented Jul 9, 2018 •

edited

Loading