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

Remove or rename the old Node.remove_and_skip() #4049

Closed
eon-s opened this issue Feb 20, 2022 · 2 comments · Fixed by godotengine/godot#64412
Closed

Remove or rename the old Node.remove_and_skip() #4049

eon-s opened this issue Feb 20, 2022 · 2 comments · Fixed by godotengine/godot#64412
Milestone

Comments

@eon-s
Copy link

eon-s commented Feb 20, 2022

Describe the project you are working on

This is not about a particular project but about Godot itself

Describe the problem or limitation you are having in your project

According to the documentation:

● void remove_and_skip()

Removes a node and sets all its children as children of the parent node (if it exists). 
All event subscriptions that pass by the removed node will be unsubscribed.

What it really does is that it fails with error if the node does not have a parent (there is a report that it does not fail but I couldn't reproduce), otherwise frees the node (not remove) then yes it leaves its children with their grandparent node.

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

Almost nobody uses this method so some options can be:

1 - Remove it and it's cursed functionality.

2 - Rename it to something meaningful like Node.free_and_reparent_children() or Node.going_out_for_cigarettes().

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

Remove or rename from the Node class.

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

No, is a core change.

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

No, there's no reason for it to be in core, that is why I ask for its removal.

@Rodeo-McCabe
Copy link

I haven't personally used it, but I imagine this method could be very useful in tool scripts that manipulate the scene tree dock or the currently edited scene in general.

I agree the name sucks though. free_and_reparent_children() is perfect, because that's exactly what it does.

@eon-s
Copy link
Author

eon-s commented Feb 20, 2022

@Rodeo-McCabe there are problems related to this function, since signals (maybe not now with callables), physics and other things may break while using it, I would prefer just a reparent_children that can be followed by a free if needed.

@Calinou Calinou added the requires core feedback Feature needs feedback from core developers label Feb 20, 2022
@Calinou Calinou changed the title Remove or rename the old remove_and_skip Remove or rename the old Node.remove_and_skip() Feb 20, 2022
@Calinou Calinou added topic:core and removed requires core feedback Feature needs feedback from core developers labels Feb 20, 2022
@akien-mga akien-mga added this to the 4.0 milestone Sep 9, 2022
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