Calling SetScript() on a node in C# causes the object to be disposed

[ajferrario]

Platform

• Windows 10
• Godot_v3.1.1-stable_mono_win64
• GTX 1050 mobile
• C# for scripting

Issue Whenever I call SetScript() on a node object it makes any future calls to that object fail with the following error.

E 0:00:03:0063   System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Godot.KinematicBody2D'.
  <C Source>     /tmp/build_GodotSharp/GodotSharp/Core/Object.base.cs:37 @ IntPtr Godot.Object.GetPtr(Godot.Object )()
  <Stack Trace>  Object.base.cs:37 @ IntPtr Godot.Object.GetPtr(Godot.Object )()
                 Node.cs:389 @ void Godot.Node.AddChild(Godot.Node , Boolean )()
                 initializePlayer.cs:27 @ void initializePlayer._on_game_ready()()

Here's the script that causes the above error. I'm trying to dynamically initialize my players for a multiplayer game and assign the player control script to the instance representing the local player.

using Godot;
using System;
using System.Collections.Generic;

public class initializePlayer : Node
{
    [Signal]
    public delegate void PlayersInitialized();

    PackedScene player = (PackedScene)ResourceLoader.Load("res://scenes/player.tscn");
    List<Node> playerInstances = new List<Node>();
    List<KinematicBody2D> players = new List<KinematicBody2D>();
    private void _on_game_ready()
    {
        Global global = GetNode("/root/Global") as Global;
        playerInstances.Add((Node)player.Instance());
        playerInstances.Add((Node)player.Instance());
        if (global.playingAsHost)
        {
            playerInstances[0].SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));
        }
        else
        {
            playerInstances[1].SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));
        }
        GetNode("/root/game/players").AddChild(playerInstances[0], true);
        GetNode("/root/game/players").AddChild(playerInstances[1], true);
        players.Add(GetNode("/root/game/players").GetChild(0) as KinematicBody2D);
        players.Add(GetNode("/root/game/players").GetChild(1) as KinematicBody2D);
        GD.Print(players[0].GetPropertyList()[0]);
        GD.Print(players[1]);
        EmitSignal(nameof(PlayersInitialized));
    }
}

It looks to me from googling around on this that somehow garbage collection believed that I was done with this object for reasons I don't understand. I've tried a number of variations on this issue and it seems to happen no matter how/where I call SetScript() or where/how I store the node object.

Repro

• Script in C#
• Create object representing node
• Call SetScript() on that node object
• Attempt any other use of that object

I have attached a sample project which demonstrates the issue I mentioned.

SetScriptIssueSample.zip

[neikeq]

This is expected behaviour. You have to be careful with SetScript as you cannot asume the managed object you are calling it on will be usable after that. If previous to the call it had a C# script attached, then it will be disposed. If you're setting it to a new C# script, then it will also be disposed.

The reason this is possible in GDScript is because internally the variable is just a Object* (as a Variant ofc). So when you replace the GDScript instance, you don't need to do anything to access the new one. You can keep using the Object*.

In C# you're not accessing the Object* directly. Instead The C# instance is a wrapper around it. When you change the C# instance, the old one is disposed so you can't use it anymore.

You can do something like this to get the new C# instance:

ulong objId = obj.GetInstanceId();
// Replaces old C# instance with a new one. Old C# instance is disposed.
obj.SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));
// Get the new C# instance
obj = GD.InstanceFromId(objId);

This is a bit verbose and the lookup by object ID adds some overhead, but it will do as a workaround for now. I'm thinking of making SetScript return the new instance to allow something like this:

obj = obj.SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));

[Jazzepi]

I wrote this C# extension to make it easier to use the snippet provided by @neikeq when loading C# scripts into nodes.

using Godot;
using Object = Godot.Object;

public static class MyExtensions
{
    public static T SafelySetScript<T>(this Object obj, Resource resource) where T : Object
    {
        var godotObjectId = obj.GetInstanceId();
        // Replaces old C# instance with a new one. Old C# instance is disposed.
        obj.SetScript(resource);
        // Get the new C# instance
        return GD.InstanceFromId(godotObjectId) as T;
    }

    public static T SafelySetScript<T>(this Object obj, string resource) where T : Object
    {
        return SafelySetScript<T>(obj, ResourceLoader.Load(resource));
    }
}

Then it can be used like this where cSharpScript is a string reference to the resource, or a strongly typed Resource object. Without the SafelySetScript calling .SetScript here will give you an object indispose error when you try to set the .Text to test.

var button = new Button();
button = button.SafelySetScript<Button>(cSharpScript);
button.Text = "test";

[tileuser]

To use @Jazzepi's C# extension for Godot 4, change Godot.Object to Godot.GodotObject and GD.InstanceFromId to Object.InstanceFromId (InstanceFromId is located under GodotObject instead of GD).

using Godot;
using Object = Godot.GodotObject;

public static class MyExtensions
{
    public static T SafelySetScript<T>(this Object obj, Resource resource) where T : Object
    {
        var godotObjectId = obj.GetInstanceId();
        // Replaces old C# instance with a new one. Old C# instance is disposed.
        obj.SetScript(resource);
        // Get the new C# instance
        return Object.InstanceFromId(godotObjectId) as T;
    }

    public static T SafelySetScript<T>(this Object obj, string resource) where T : Object
    {
        return SafelySetScript<T>(obj, ResourceLoader.Load(resource));
    }
}

[todeskurve]

This is expected behaviour. You have to be careful with SetScript as you cannot asume the managed object you are calling it on will be usable after that. If previous to the call it had a C# script attached, then it will be disposed. If you're setting it to a new C# script, then it will also be disposed.

The reason this is possible in GDScript is because internally the variable is just a Object* (as a Variant ofc). So when you replace the GDScript instance, you don't need to do anything to access the new one. You can keep using the Object*.

In C# you're not accessing the Object* directly. Instead The C# instance is a wrapper around it. When you change the C# instance, the old one is disposed so you can't use it anymore.

You can do something like this to get the new C# instance:

ulong objId = obj.GetInstanceId();
// Replaces old C# instance with a new one. Old C# instance is disposed.
obj.SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));
// Get the new C# instance
obj = GD.InstanceFromId(objId);

This is a bit verbose and the lookup by object ID adds some overhead, but it will do as a workaround for now. I'm thinking of making SetScript return the new instance to allow something like this:

obj = obj.SetScript(ResourceLoader.Load("Scripts/playerMovement.cs"));

if the SetScript-method instantiates the Node, the old id, should still point to the old (now disposed) instance and the new id is unknown to us. where am i wrong here?

is there any reason the SetScript does not change reference we called it upon to the new instance? i cant imagine where that could make problems and we wont have to deal with: node = node.SetScript(...)

PS: i use the path to get the new instance:

            string oldPath = node.GetPath();
            node.SetScript(GD.Load<Script>(scriptPath));
            node = otherNonDisposedNode.GetNode(oldPath);