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

Execute a C# Script from command line with C# Autoload results in two different SceneTree instances #62101

Closed
avilches opened this issue Jun 16, 2022 · 6 comments

Comments

@avilches
Copy link
Contributor

Godot version

3.4.3.stable.mono

System information

macOS Monterey 12.4

Issue description

Execute a Godot application using the -s parameter destroys previous SceneTree created before the script is executed. This can be noticed if the application has autoload nodes and they access to GetTree() in the _Ready() method, which happens before the -s script is created.

It looks like Godot is creating a new SceneTree instance using the -s class which is totally different than the SceneTree instance returned from the GetTree() method in the _Ready() methods from any autoload object, instead of return the same instance. This is not happening after the _Initialize() method, so, calling to GetTree() in _Process() method from any the autoload object return the right instance. But the previous wan has been disposed.

Flow:

  1. Godot is started using -s script, which inherits from SceneTree
  2. Godot adds all the autoloads to the SceneTree.Root and wait until they are ready.
  3. So, any call to GetTree() method from autoloads _Ready() returns a SceneTree. We will called it SceneTree A.
  4. New SceneTree B is created using the -s parameter.
  5. The SceneTree A is disposed.
  6. Since now, any GetTree() call from autoloads returns the B instance and any access to the A instance will fail.

Steps to reproduce

  1. Create this autoload and add it to the project.godot:
[autoload]
Autload1="*res://AutoloadNode2D.cs"
public class AutoloadNode2D : Godot.Node2D {
    private SceneTree _sceneTreeFromReady;

    public override void _Ready() {
        _sceneTreeFromReady = GetTree();
    }

    public override void _Process(float delta) {
        // They are different instances, different hash code and the previous one is disposed
        Console.WriteLine("GetTree().GetHashCode():" + GetTree().GetHashCode());
        Console.WriteLine("_sceneTreeFromReady.GetHashCode():" + _sceneTreeFromReady.GetHashCode());
        Console.WriteLine("IsInstanceValid(_sceneTreeFromReady):" + IsInstanceValid(_sceneTreeFromReady));
        // Its not valid, so this crashes
        _sceneTreeFromReady.GetFrame();
    }
}
  1. Create a SceneTree script
public class Script : SceneTree {}
  1. Execute it with
godot -s Script.cs
  1. This log is shown
_sceneTreeFromReady.GetHashCode():633486861
GetTree().GetHashCode():601709270
IsInstanceValid(_sceneTreeFromReady):False

Unhandled Exception:
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Godot.SceneTree'.
  at Godot.Object.GetPtr (Godot.Object instance) [0x0001b] in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Object.base.cs:43 
  at Godot.SceneTree.GetFrame () [0x00001] in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Generated/GodotObjects/SceneTree.cs:487 
  at AutoloadNode2D._Process (System.Single delta) [0x0006a] in /Users/avilches/Godot/test/AutoloadNode2D.cs:16 

Minimal reproduction project

scene-tree-script-with-autoload-error.zip

@raulsntos
Copy link
Member

It's actually the same SceneTree instance, you can check that GetInstanceId returns the same ID for both. What's happening here is that when you call GetTree, C# is marshaling that instance and it results in 2 different C# instances.

The reason why C# returns different instances when marshaling is that when autoloads execute their _Ready method, the SceneTree has not been initialized yet (you can check by overriding _Initialize in your Script class).

Since the SceneTree hasn't been initialized, when trying to get the managed type from the unmanaged type, we are unable to retrieve a tied managed instance from a CSharpInstance script instance. What this means is that we haven't created an instance of Script (the script attached to the node, the node itself is already created).

Instead, it retrieves the native instance binding which is just an instance of SceneTree. Basically the same node but without the script attached.

Later, the script instance is actually created so we can retrieve the tied managed instance and it results in a different instance in C#.

So to resolve this issue we would could implement either of these two options:

  • Delay adding the autoloads to the SceneTree until after initialization.
  • Attach the script to the SceneTree much earlier so it happens before the autoloads are added to the tree.

I'm not sure which option is preferred.


The autoloads are added to the scene in Main::start:

godot/main/main.cpp

Lines 3058 to 3060 in a83eb16

for (Node *E : to_add) {
sml->get_root()->add_child(E);
}

The SceneTree is initialized by the OS after calling Main::start:

if (Main::start()) {
os.set_exit_code(EXIT_SUCCESS);
os.run(); // it is actually the OS that decides how to run
}

main_loop->initialize();


Either way this would require changes to Core, I don't think there's anything we can do in the .NET module to resolve this.

@avilches
Copy link
Contributor Author

To me, the autoloads are user code, so they shouldn't be added until everything is ready in the core. The ideal will be just after the main_loop->initialize(). The problem is even worse, because this bootstrap is slightly different in every os, so the core should be updated in os_windows, os_android, os_linuxbsd.... @reduz @akien-mga

@raulsntos
Copy link
Member

This issue seems like it may be a duplicate of:

There are already some open PRs that fix those issues:

I haven't looked too deep but if you feel like those already cover this issue we can close as duplicate and you can track progress there.

@avilches
Copy link
Contributor Author

Thanks, I've subscribed to them, so, as soon as they were merged, I can test this specific issue and close it as solved.

@raulsntos
Copy link
Member

The mentioned PRs have been merged and are included in 4.2-dev.1. Can you check if the issue still exists?

@avilches
Copy link
Contributor Author

avilches commented Aug 6, 2023

The mentioned PRs have been merged and are included in 4.2-dev.1. Can you check if the issue still exists?

Tested in godot-4-2-dev-2, it worked like a charm. :)

@avilches avilches closed this as completed Aug 6, 2023
@akien-mga akien-mga added this to the 4.2 milestone Aug 9, 2023
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

4 participants