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 Position* Nodes to Marker* #5199

Closed
Mickeon opened this issue Aug 18, 2022 · 7 comments · Fixed by godotengine/godot#64370
Closed

Rename Position* Nodes to Marker* #5199

Mickeon opened this issue Aug 18, 2022 · 7 comments · Fixed by godotengine/godot#64370
Labels
breaks compat Proposal will inevitably break compatibility topic:2d topic:3d
Milestone

Comments

@Mickeon
Copy link

Mickeon commented Aug 18, 2022

Describe the project you are working on

Typical 2D Godot game, taking a look at Godot 4.

Describe the problem or limitation you are having in your project

Godot 4 changes a lot of Node names to be more explicit and explanatory. Despite this, through the transition, one Node feels like has been left out...

The name of Position2D and Position3D is beyond unusual.
Firstly, It's vague! What is it? The Node that has a position? Don't all 2D/3D nodes have that?
Indeed, it shares its name with one of its properties. Thus, assuming its name is unchanged, when fetching the position (you're going to probably do that to make use of its specialty), it's totally possible to be in this situation:

@onready var position_2d := $Position2D
...
position_2d.position += Vector2.ONE

Needless to say, this is very confusing, especially for learning users that have yet to properly grasp the concept of Nodes.

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

As brought up in godotengine/godot#54161 (comment).

Rename these nodes to Marker. Therefore:

  • Position2D -> Marker2D
  • Position3D -> Marker3D

This name is much simpler and suits their purpose to a tea. They're Nodes, used to mark noteworthy spots. The icon fits much better, too!

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?

It cannot. At least, not properly. One can just live with the current name, as is, and rename every Position node to to Marker on their own.

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

Position2D and Position3D are core classes.

@Mickeon Mickeon changed the title Rename Position* Nodes to Marker Rename Position* Nodes to Marker* Aug 18, 2022
@mieldepoche
Copy link

I would be happy with Marker instead of Position.

But why not remove Markers in favor of having a toggle on Node2D/Node3D to toggle the display of the axes instead?
The fact that the marking feature ("this point is important") is separated in inheritance from the transform feature ("this point exist") is weird imo.

@aaronfranke aaronfranke added this to the 4.0 milestone Aug 18, 2022
@aaronfranke
Copy link
Member

aaronfranke commented Aug 18, 2022

I think this is a good idea. I thought about it before, since the name Position is indeed confusing, and the name that came to mind at the time was Gizmo2D and Gizmo3D. However, I like the name Marker a lot better. 👍

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

It cannot.

To be clear, the work-around is to just deal with the old name, or you can also do const Marker3D = Position3D to alias the type on a per-script basis. So this question on the proposal wasn't quite answered correctly.

@Mickeon
Copy link
Author

Mickeon commented Aug 18, 2022

Edited the proposal accordingly.

@aaronfranke aaronfranke added the breaks compat Proposal will inevitably break compatibility label Aug 18, 2022
@Riteo
Copy link

Riteo commented Aug 20, 2022

I would be happy with Marker instead of Position.

But why not remove Markers in favor of having a toggle on Node2D/Node3D to toggle the display of the axes instead? The fact that the marking feature ("this point is important") is separated in inheritance from the transform feature ("this point exist") is weird imo.

Nah, the base nodes should stay as pure and clean as possible IMO. Marker*/Position* nodes have a clearly defined scope: marking positions.

Still, you could open a proposal if you care.

@Diddykonga
Copy link

Diddykonga commented Aug 22, 2022

Nah, the base nodes should stay as pure and clean as possible IMO. Marker*/Position* nodes have a clearly defined scope: marking positions.

I just want to point out, this is not a clearly defined purpose, and personally I would remove these as they are a waste of a class name. Every Node2D/Node3D already has an transform and therefore a position.

As suggested if the debug/visual information is the only unique thing, and could be relevant for other classes to also posses, then it should just be added to Node2D/Node3D. It seems like these types of Nodes are relics of the past, and instead all proper Nodes should have Debug functionality implemented for them and exposed to the Editor for developers to use.

@Mickeon
Copy link
Author

Mickeon commented Aug 22, 2022

The class could be firstly and foremost renamed, and then expanded upon with more features.

@Diddykonga
Copy link

The class could be firstly and foremost renamed, and then expanded upon with more features.

If we wish to keep the Node, then I suggest what @aaronfranke mentioned above, with Gizmo2D/3D.
An editor-only node for custom Gizmo creation/display, although it would need to be quite complex to support all the different types of Gizmos developers could want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:2d topic:3d
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants