godot icon indicating copy to clipboard operation
godot copied to clipboard

Duplicating a node tree with a @tool script that edits global_position/global_rotation causes nodes to lose their transforms

Open Jamsers opened this issue 1 year ago • 7 comments

Tested versions

  • Reproducible in: v4.2.1.stable

System information

Godot v4.2.1.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Laptop GPU (NVIDIA; 31.0.15.5176) - AMD Ryzen 5 5600H with Radeon Graphics (12 Threads)

Issue description

Duplicating a node tree with a @tool script that edits global_position/global_rotation causes nodes to lose their transforms. A spam of scene/3d/node_3d.cpp:343 - Condition "!is_inside_tree()" is true. Returning: Transform3D() errors print in the console.

https://github.com/godotengine/godot/assets/39361911/9985cc7f-d814-413c-a53a-0601109b8230

Steps to reproduce

Create a Node3D, with a set of Node3D children Attach a @tool script to the children that has a function that changes global_position/global_rotation Create a exported bool "button" that allows the function to be called from the inspector, such as @export var function: bool = false : set = function_exec Duplicate the Node3D

Minimal reproduction project (MRP)

Script_Position_Bug.zip

Jamsers avatar Mar 24 '24 13:03 Jamsers

Same for me. After I instantiated a PackedScene, the resulting children from that new node are (according to the errors) not in the tree.

EDIT: Not only in @tool scripts, also while the game is running.

Max-Beier avatar Mar 24 '24 14:03 Max-Beier

I checked. Here you call the function_exec method after you create the node but before it is in the tree. In this function, the global_position method requires the node to be in the tree. If you need something like this example you gave you can use this:

@tool
extends Node3D

@export var function: bool = false : 
	set(value): if is_inside_tree(): function_exec

func function_exec(dummy: bool) -> void:
	global_position = global_position

Edit: Also works if you duplicate only one Node3D. Or instantiating the same Node3D.

Nazarwadim avatar Mar 24 '24 14:03 Nazarwadim

@Max-Beier You probably also used a method that requires the node to be in the tree. Just add it to the tree with the add_child method before that method

Nazarwadim avatar Mar 24 '24 15:03 Nazarwadim

@Nazarwadim I did, but only the parent. Do I need to add the children separately as well?

Max-Beier avatar Mar 24 '24 17:03 Max-Beier

Do I need to add the children separately as well?

No, you didn't. Probably you call method when instantiating. I can't say for sure because I don't have more information about your case.

Nazarwadim avatar Mar 24 '24 18:03 Nazarwadim

Here's the relevant part, I don't think there's anything wrong


var sections: Array[Section] = []

func _load_types():
	var path := "res://objects/sections/types/"
	var dir := DirAccess.open(path)
	
	types = []
	sections = []
	if not dir:
		printerr("(Section) Types directory does not exist.")
		return
	
	dir.list_dir_begin()
	var file_name := dir.get_next()
	while file_name != "":
		var scene: PackedScene = load(path + file_name)
		var section: Section = scene.instantiate()
		
		for socket in section.sockets:
			socket.set_meta("connected", false)
		
		if !section.sockets.is_empty():
			types.push_back(section)
		file_name = dir.get_next()
		

func _generate():
	if !centre_position || centre_position.distance_to(player.position) > render_distance:
		centre_position = player.position
		
		if sections.is_empty():
			var type := types[rng.randi() % types.size()]
			var section: Section = type.duplicate()
			section.position = centre_position
			add_child(section)
			sections.push_back(section)
		
		for section in sections:
			if centre_position.distance_to(section.position) > render_distance:
				sections.erase(section)
				section.queue_free()
				continue
			
			for socket in section.sockets:
				if socket.get_meta("connected"):
					continue
				
				var type := types[rng.randi() % types.size()]
				var new_section: Section = type.duplicate()
				add_child(new_section)
				sections.push_back(new_section)
				
				var new_socket = new_section.sockets[rng.randi() % new_section.sockets.size()]
				new_section.global_position = section.global_position + (socket.global_position - new_socket.global_position)
				
				new_socket.set_meta("connected", true) 
				socket.set_meta("connected", true) 

Max-Beier avatar Mar 24 '24 19:03 Max-Beier

  1. Which line is causing the error? Perhaps here:
		var section: Section = scene.instantiate()
		
		for socket in section.sockets:
			socket.set_meta("connected", false)
		
		if !section.sockets.is_empty():
  1. This is not related to the error before, but never use DirAccess to access res://. It should be used for user://. If you need to, you can see more details here on how to do it correctly: https://youtu.be/4vAkTHeoORk?t=4377

Nazarwadim avatar Mar 24 '24 20:03 Nazarwadim

This all works fine, the error is when I try to access the global_position of the sockets (the children) of the section.

				new_section.global_position = section.global_position + (socket.global_position - new_socket.global_position)

(new_section.global_position & section.global_position are working)

Max-Beier avatar Mar 25 '24 09:03 Max-Beier