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

Rename properties for Color() #2092

Closed
CitrusWire opened this issue Jan 9, 2021 · 12 comments
Closed

Rename properties for Color() #2092

CitrusWire opened this issue Jan 9, 2021 · 12 comments

Comments

@CitrusWire
Copy link

Describe the problem or limitation you are having in your project

The Color() method has a bunch of properties which are all one single character:
https://docs.godotengine.org/en/latest/classes/class_color.html

This is terrible. You shouldn't do this in personal code; you absolutely shouldn't do it in a public API except where there's unanimous understanding (i.e.: x, y for Vectors).

Worse it's not even being consistent in its use of them. "b" means blue as a property; I'm guessing it doesn't mean that in linear_interpolate(b,t)?

A user should be able to glance at their code and instantly know what it does, not wonder what Color.v represents.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Properties with descriptive names. This includes in the methods (linear_interpolate(b,t); b and t == ?)

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

green
blue
red
alpha

etc...
@Jummit
Copy link

Jummit commented Jan 9, 2021

I agree that h, s and v are not that obvious, but when I read enemy.color.r, I think there is no confusion about what it stands for, especially because this is how it works in shaders, too. The methods have descriptions that explain what the arguments mean, so I'm not sure if they need to be longer.

@CitrusWire
Copy link
Author

CitrusWire commented Jan 9, 2021

when I read enemy.color.r, I think there is no confusion about what it stands for

I can guess what it stands for, but you should never have to guess. With experience a user can learn, but they shouldn't have to learn this sort of thing. All its doing is further increasing the cognitive load on the dev for the sake of typing a tiny number of extra characters.

@YuriSizov
Copy link
Contributor

I agree that h, s and v are not that obvious

And even then, these are components of the common color format which is referred to as HSV in most of the literature. So I think if you are going to bother using HSV, you would know what Color.v stands for at a glance. Same as most people know what Color.b stands for.

That said, I don't see why we can't introduce aliases for all of them for a bit more explicitness?

@CitrusWire
Copy link
Author

That said, I don't see why we can't introduce aliases for all of them for a bit more explicitness?

I'd suggest deprecating the single characters and only keeping the full words. It's much better practice.

If Godot is fine with single characters for this, why not for other "obvious" things too? Before you know it you have an abstruse collection of properties and methods (so just like the linux command line...). If Godot is to be the "Easy to use" is sells itself as, it shouldn't go down that route.

(This applies to any other classes/methods that have such single-character properties that are not either x, y, z or some other ubiquitous convention.)

@Calinou
Copy link
Member

Calinou commented Jan 9, 2021

The GLSL/Godot shader language types use r/g/b/a too, and these have been standard for decades there. I don't see much reason to change the Color property names either.

@aaronfranke
Copy link
Member

aaronfranke commented Jan 10, 2021

In most code, having a property be a single letter is a bad idea and is best avoided because it's not very verbose or discoverable.

However, the core data structures are frequently used enough that the above does not apply. It is a reasonable expectation that users should know what the members of a Color or Vector2 are, and it shouldn't require any additional verbosity to explain that .r on a color means the red channel and .x on a Vector2 refers to a coordinate that goes left/right. Someone who has been using Godot for awhile should have the basic structures memorized because they're needed so often.

(Aside from that, there is a very huge argument for not changing it because of existing/standardized/historical usage, but I think the argument for keeping these as r/g/b/a stands on its own even without that).

That said, I don't see why we can't introduce aliases for all of them for a bit more explicitness?

To quote reduz on the topic of having two APIs for the same thing:

@YuriSizov
Copy link
Contributor

To quote reduz on the topic of having two APIs for the same thing

And he is correct in the ideal world, but we already have aliases for some methods at least, and I am not sure that all of them are getting removed for 4.x API clean up. Also, in shaders everything is a vec and whether you access XYZ or RGBA or I think a couple more options is based on the context you want, so it's not like aliases are foreign for Godot and game development 🙂

A better argument against aliases would be potential conflicts with color names (in case of RGB). HSV is not vulnerable to that though.

@CitrusWire
Copy link
Author

.x on a Vector2 refers to a coordinate that goes left/right

Agreed, because it's ubiquitous across most of society. Which is why I'm only talking about Color here.

Someone who has been using Godot for awhile should have the basic structures memorized because they're needed so often.

What you're proposing is making things more difficult than they need to be... why?

Lets look at it this way:

Benefits of current method:

  • Saves typing a tiny number of characters
  • Historical
  • Consistent with GLSL

Detriments:

  • Increases cognitive load when parsing code
  • Something else for newbies to remember (and forget, and re-remember, and re-forget...)
  • Exhibits bad practices

(Missing any from either list?)
To me the benefits are far outweighed by the detriments. Especially given 4.x is coming along which is the perfect time to deprecate stuff and that obviates the Historical issue to some degree.

Godot sells it self as "easy to use". Putting up what are effectively arbitrary barriers is the opposite of that. And yes, you can say "but it's only a few characters", and you're right, but I've begun to notice that Godot has lots of these little barriers and they add up.

@YuriSizov
Copy link
Contributor

Being easy (or rather easier) to use doesn't mean that it comes without any cognitive load. There are things that people need to learn which won't come naturally. This is not a game constructor type of engine, it doesn't abstract all of the technical stuff for the sake of making a kid friendly UX. You still have to learn about its domain and peculiarities of gamedev. Because that's the only way to reach proficiency and not sacrifice flexibility.

Godot doesn't claim to be anything else.

@dalexeev
Copy link
Member

The proposed change conflicts with the constants.



Of course, we can rename the constants to RED, GREEN, BLUE, etc, but someone would still say that red and RED are causing confusion and that red_channel would be clearer. rredred_channel. The more clarity, the longer the names.

I think because there are relatively few such properties in the API (x, y, z, w, h, r, g, b, a), this is not a big problem.

@aaronfranke
Copy link
Member

@dalexeev The constants will almost certainly be renamed in 4.0. Either to Color.RED or Colors.RED.

@Calinou
Copy link
Member

Calinou commented Nov 3, 2021

Closing due to lack of support (see reactions on OP).

For future reference, color constants were renamed to uppercase in godotengine/godot#45144 for Godot 4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants