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 Control's Rect properties to exclude "rect_" part #3125

Closed
SilencedPerson opened this issue Aug 12, 2021 · 8 comments · Fixed by godotengine/godot#57095
Closed

Rename Control's Rect properties to exclude "rect_" part #3125

SilencedPerson opened this issue Aug 12, 2021 · 8 comments · Fixed by godotengine/godot#57095
Milestone

Comments

@SilencedPerson
Copy link

SilencedPerson commented Aug 12, 2021

Describe the project you are working on

A game using Control node

Describe the problem or limitation you are having in your project

Everytime I try to modify the control node with code, i end up forgetting the "rect_" part, it's extremely annoying.
It is weirdly inconsistent with other 2D nodes and makes me question why viewport node and the shape inside CollisionShape2D don't have the same treatment.

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

Just remove the "rect_" part, it's redundant.

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

image

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, it cannot be done with a plugin.

Is there a reason why this should be core and not an add-on in the asset library?

This is about unifying the property naming convention with the rest of the engine.

@SilencedPerson
Copy link
Author

It was also discussed if the Node3D's translation should be changed to position with an intent to mimic Node2D on which the users agreed that it should, I don't know if that change was greenlit by the devs of the engine, but if it was, then this one should be too, since it's basically same thing.

@Calinou
Copy link
Member

Calinou commented Aug 12, 2021

It was also discussed if the Node3D's translation should be changed to position with an intent to mimic Node2D on which the users agreed that it should, I don't know if that change was greenlit by the devs of the engine, but if it was, then this one should be too, since it's basically same thing.

Node3D's translation was renamed to position in godotengine/godot#44198.

@Xrayez
Copy link
Contributor

Xrayez commented Aug 12, 2021

I think one of the reasons why those properties are prefixed with rect_ is because they could be automagically grouped in the editor inspector. But as far as I know, it's not a requirement, and properties can be grouped together regardless of a prefix with ADD_GROUP() in the engine code.

@mrtripie
Copy link

I'd like this change too. I've been doing a lot UI of lately and I forget the rect part every time.

@SilencedPerson
Copy link
Author

I think one of the reasons why those properties are prefixed with rect_ is because they could be automagically grouped in the editor inspector. But as far as I know, it's not a requirement, and properties can be grouped together regardless of a prefix with ADD_GROUP() in the engine code.

If this is legitmate concern, i would prefer it to be consistent among other nodes too.
instead of position, translation and rect_position => node_position for all of them.

@Calinou
Copy link
Member

Calinou commented Aug 26, 2021

But as far as I know, it's not a requirement

It's indeed not a requirement, so we don't need to rename Node2D properties 🙂

@SilencedPerson
Copy link
Author

SilencedPerson commented Sep 30, 2021

did anyone attempt to do something with this? i currently figuring out how to go about implementing it.
it seems to me like it should be easy, just rename it in few places but i have no frame of reference.

@Calinou Calinou added this to the 4.0 milestone Sep 30, 2021
@Calinou
Copy link
Member

Calinou commented Sep 30, 2021

did anyone attempt to do something with this?

Not that I know of. Feel free to open a pull request against the master branch 🙂

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

Successfully merging a pull request may close this issue.

4 participants