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

Nodes in the "Remote" window do not match the node name #92687

Closed
JekSun97 opened this issue Jun 2, 2024 · 13 comments
Closed

Nodes in the "Remote" window do not match the node name #92687

JekSun97 opened this issue Jun 2, 2024 · 13 comments

Comments

@JekSun97
Copy link

JekSun97 commented Jun 2, 2024

Tested versions

I checked version 4.2.2 and 4.3 beta1, in 3.5.3 this problem is missing

System information

Godot v4.3.beta1 - Windows 10.0.19045 - GLES3 (Compatibility) - Radeon RX 560 Series (Advanced Micro Devices, Inc.; 31.0.14001.45012) - Intel(R) Core(TM) i5-4570 CPU @ 3.20GHz (4 Threads)

Issue description

When scenes are dynamically created, their names do not correspond to their name, instead they use the name of the node type.

3.5.3:
3 5

4.x:
4 plus

Steps to reproduce

  1. Create a node with a non-standard name and save it.
  2. Load this node in any way and add it to the scene as a child node using code.
  3. Open the "Remote" tab while the game is running, make sure that the names of the created nodes do not match their names in the file manager.

Minimal reproduction project (MRP)

test_for 4.3 beta1.zip

@timothyqiu
Copy link
Member

timothyqiu commented Jun 2, 2024

This is intended behavior (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.

Pass true for the second parameter if you want the adjusted name to be based on the original one.


How the @ names are generated is an implementation detail I believe.

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

@timothyqiu The second argument in my test project doesn't change anything.

In cases where this problem is fixed, I would suggest changing the second argument to true by default. I often use "Remote" in my projects, and searching by name is quite logical, rather than by type.

32

@AThousandShips
Copy link
Member

I would suggest changing the second argument to true by default.

This would break compatibility so won't be possible, it's also undesirable for default performance

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

Please. Demonstration of the problem with the second argument.
test3.zip

@AThousandShips
Copy link
Member

What problem? It worked according to your previous comment? Are you talking about the default value?

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

What problem? It worked according to your previous comment? Are you talking about the default value?

Perhaps I am not sufficiently informed about the big changes in the fourth version, and to be honest I’m surprised by this behavior of “Remote”, but I write add_child(scn,true) and add_child(scn,false), nothing changes, the name remains the name of the node type

@AThousandShips
Copy link
Member

AThousandShips commented Jun 2, 2024

The node is just named Node2D so what's wrong here? It's named what the node is named?

[node name="Node2D" type="Node2D"]

You've not named it Test like in 3.x

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

The node is just named Node2D so what's wrong here? It's named what the node is named?

[node name="Node2D" type="Node2D"]

You've not named it Test like in 3.x

32

Run my project, make sure its name is not "Node2D"

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

I'm sorry, I renamed the file itself, but not the node name, my fault...

Maybe we should add an option to the project settings that adds all nodes readable in "Remote" by default, for testing?

@AThousandShips
Copy link
Member

AThousandShips commented Jun 2, 2024

That would be a lot of extra fragile code instead of just using the function properly, I'm sorry but this isn't really a good change, it's well documented and can't be changed for compatibility, and a project setting would be a significant cost to performance and be confusing

I'd suggest making a proposal if you really want that change, but I doubt it'll be approved, otherwise it's an easy mistake and it's solved by reading the documentation and discovering how to use things like intended

@JekSun97
Copy link
Author

JekSun97 commented Jun 2, 2024

I'm not very well versed in the source code, but this is only one condition, checking the project settings for add_child, compatibility would not be broken if this setting was not found in the project settings, the usual add_child would be used as before

If I migrate my entire old project to version 4, I will have to rewrite all the add_child functions in order to find them quickly in "Remote" and change them for testing

and when the project passes the test, all these functions will again have to be rewritten with a negative second argument for performance

@AThousandShips
Copy link
Member

AThousandShips commented Jun 2, 2024

It would hurt performance since you'd have to check the settings each time the method is called, and it'd be confusing that the argument to the method is ignored if the setting is set, that's not done anywhere else

The desired default is to not make the name readable, as explained in the documentation description, if you want it the simple solution is to just set the argument to true, this is not a big task

Further the usage to test things is not really generally how this would be used, you don't have to change the condition back and forth like that, that's not how most people use this AFAIK

But I could be wrong about how likely this is to be relevant, so please open a proposal if you want that feature and that'll let us know if others have the same need for this

Edit: What do you mean by migrating your project? The argument default is the exact same in 3.x as in 4.x?

@AThousandShips
Copy link
Member

Closing in favor of:

Thank you for your contribution

@AThousandShips AThousandShips closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2024
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

3 participants