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

C# Getting Node3D Position/GlobalPosition in Thread will always return zero #79164

Closed
Xyotic opened this issue Jul 7, 2023 · 7 comments
Closed

Comments

@Xyotic
Copy link

Xyotic commented Jul 7, 2023

Godot version

Godot 4.1 stable

System information

Windows 10, RTX2080, i9-9900K

Issue description

Trying to access the position with node3D.Position or node3D.GlobalPosition inside a thread will always return Vector3(0,0,0) in 4.1
Worked without any issue in 4.0

Steps to reproduce

Add a Node to your scene and attach a script to it like this: (same code in mrp)

using Godot;
using System;
using System.Threading.Tasks;

public partial class Demonstration : MeshInstance3D
{
	public override void _Ready()
	{
		var currentPosition = this.Position;
		GD.Print(currentPosition); // will print the correct position
		
		Task.Run(async() => 
		{
			await Task.Delay(100);
			var currentPositionInThread = this.Position;
			GD.Print(currentPositionInThread); // will print zero
		});
	}
}

If step into this code you will see that the information of the position will be lost as soon as you enter the Task.Run() block

Minimal reproduction project

GetPositionIssue.zip

@raulsntos
Copy link
Member

raulsntos commented Jul 7, 2023

Accessing node members outside of the main thread is not allowed. The Node3D::get_position method has a thread guard:

godot/scene/3d/node_3d.cpp

Lines 508 to 511 in c3b0a92

Vector3 Node3D::get_position() const {
ERR_READ_THREAD_GUARD_V(Vector3());
return data.local_transform.origin;
}

You should be getting this error:

ERROR: This function in this node (/root/Node3D/MeshInstance3D) can only be accessed
       from either the main thread or a thread group. Use call_deferred() instead.

Take a look at the Thread-safe APIs documentation for more information.


You can follow the error's suggestion and use CallDeferred or Callable.CallDeferred to avoid this error, here's an example using a Callable:

using Godot;
using System;
using System.Threading.Tasks;

public partial class Demonstration : MeshInstance3D
{
	public override void _Ready()
	{
		var currentPosition = this.Position;
		GD.Print(currentPosition); // will print the correct position
		
		Task.Run(async() => 
		{
			await Task.Delay(100);
			Callable.From(() =>
			{
				var currentPositionInThread = this.Position;
				GD.Print(currentPositionInThread); // will print the correct position
			}).CallDeferred();
		});
	}
}

@Xyotic
Copy link
Author

Xyotic commented Jul 7, 2023

Thanks for clarification.
This change in 4.1 breaks a lot of my code.
Is there any way I can await getting the nodes position? i.e. awaiting CallDeferred?

@raulsntos
Copy link
Member

raulsntos commented Jul 7, 2023

Yes, you should also be able to await the process_frame signal on the SceneTree:

await ToSignal(GetTree(), SceneTree.SignalName.ProcessFrame);

The continuation will execute in the main thread.

@RedworkDE
Copy link
Member

Also consider if the Task.Run is even necessary, for example _Ready could be made async directly:

public override async void _Ready()
{
	var currentPosition = this.Position;
	GD.Print(currentPosition); // will print the correct position
	
	await Task.Delay(100);
	var currentPositionAfterWait = this.Position;
	GD.Print(currentPositionAfterWait); // will print the correct position
}

@Xyotic
Copy link
Author

Xyotic commented Jul 7, 2023

Also consider if the Task.Run is even necessary, for example _Ready could be made async directly:

public override async void _Ready()
{
	var currentPosition = this.Position;
	GD.Print(currentPosition); // will print the correct position
	
	await Task.Delay(100);
	var currentPositionAfterWait = this.Position;
	GD.Print(currentPositionAfterWait); // will print the correct position
}

Would this not block the whole game? Considering you are awaiting on the main thread?

@RedworkDE
Copy link
Member

RedworkDE commented Jul 7, 2023

That is the whole point of async, awaiting will not block the thread, instead it can continue doing other stuff.
See https://learn.microsoft.com/en-us/dotnet/csharp/asynchronous-programming/async-scenarios

@Xyotic
Copy link
Author

Xyotic commented Jul 7, 2023

Alright thanks for your suggestions @raulsntos & @RedworkDE
I will give those a shot
Closing this as this isnt a bug

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