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

TileMap is incorrectly setting child scene's unique node names #82559

Closed
TimCoraxAudio opened this issue Sep 30, 2023 · 5 comments
Closed

TileMap is incorrectly setting child scene's unique node names #82559

TimCoraxAudio opened this issue Sep 30, 2023 · 5 comments

Comments

@TimCoraxAudio
Copy link

TimCoraxAudio commented Sep 30, 2023

Godot version

4.1.1.stable

System information

Godot v4.1.1.stable - Ubuntu 23.04 23.04 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1070 (nvidia; 535.113.01) - AMD Ryzen 5 1600 Six-Core Processor (12 Threads)

Issue description

When using scene placement in a tile set with the Scenes Collection only one of a given node placed will have it's proper node name set. The rest of the nodes of the same type will have a unique node name that is set to the inherited node name, not the actual given node name. e.g. a node inheriting from Node2D named "NodeName" will be set to "@Node2D@2" instead of "NodeName2".

This is working as expected in 4.0-stable

Steps to reproduce

Create a new Sprite2D node, assign some sprite and rename the root node to something other than "Sprite2D" and save it as a scene. Then create a new scene with a TileMap, create a TileSet resource for it, add a Scenes Collection for it and add the previously created node into it. Then draw a few of the nodes onto the TileMap , hit run and inspect the node names in the remote scene tree

Minimal reproduction project

example.zip

@jackwilsdon
Copy link
Contributor

This was changed in #75760 by the looks - it's not immediately apparent if that was intentional. You probably don't want to be depending on the node name in the first place if you can avoid it though - I don't know if auto-generated node names come with any real guarantees.

@TimCoraxAudio
Copy link
Author

I still wouldn't think it would be intentional. The name is fairly wrong.

Why wouldn't node names be reliable? They should follow some logical naming conventions

@rakkarage
Copy link
Contributor

rakkarage commented Oct 27, 2023

image

I think this intentional. Seems logical? Same thing happens when:

extends Node

var example_scene = preload("res://example.tscn")

func _ready():
	for i in range(20):
		var instance = example_scene.instantiate() // called instance() in older versions
		add_child(instance)

You can iterate with index and maybe get a pointer to tile associated scene from the TileMap without worry about name?
What would you gain by having them all named the same if that was possible? Which it is not.
Or how would u like them named?

@dalexeev
Copy link
Member

See Node.add_child():

If force_readable_name is true, improves the readability of the added node. If not named, the node is renamed to its type, and if it shares name with a sibling, a number is suffixed more appropriately. This operation is very slow. As such, it is recommended leaving this to false, which assigns a dummy name featuring @ in both situations.

ans Node.name:

Note: Auto-generated names might include the @ character, which is reserved for unique names when using add_child. When setting the name manually, any @ will be removed.

@TimCoraxAudio
Copy link
Author

Sorry for the late reply.

This was changed in #75760 by the looks - it's not immediately apparent if that was intentional. You probably don't want to be depending on the node name in the first place if you can avoid it though - I don't know if auto-generated node names come with any real guarantees.

This certainly makes more sense now with what @dalexeev has mentioned. Looks as though I'll just have to live with it

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

5 participants