Mono: SetScript leaves initial node references in weird invalid state

[Flavelius]

Godot version: 3.2.1.stable.mono.official

Issue description: Using SetScript on a node, earlier references to that node still have the old type, but newly retrieved referenes to the same node are of the new type. When trying to access node members from the initial ref that are not of the custom type (engine internal) an exception is thrown, but members from the initial custom type on the same ref can still be accessed.

Steps to reproduce: Run the attached project and examine the output log and NodeTester.cs

Minimal reproduction project: TypeWeirdness.zip

[31]

Is this explained by https://github.com/godotengine/godot/issues/31994#issuecomment-570073343 ?

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. [...]

(I happened to see your issue and the old one together in a random issue search I was doing, and figured I should try to help connect the dots if I can. 🙂 Sorry if this isn't helpful.)

[Flavelius]

I assumed that it worked like that, yes. The problem (what both issues are similar in) is that this goes agains conventions and expectations. In unity, testing myObj == null (they override ==) would atleast return true if something happened to the unmanaged side and one could work with this, but godot doesn't have this and just 'lies' about the object not being null, or having magically changed type - the second is what my issue is about which is something that's not even possible in c#/.net by default. Only by accessing the object then it actually throws an exception. My suggestion is implementing a similar workaround. Because it's quite inconvenient to test if every reference is of type Godot.Object, and if it is to use IsInstanceValid() but else use normal null checking (==null) if one just needs to now if a reference is valid. This would also at the very least give an easy option to guard against the issue reported here, because if i get a disposedexception at first, i can take that as a basis to implement a simple nullcheck (and be informed that the previous reference is now invalid).

[Mupli]

I have this problem now .. and feels like this could be solved by "proxy object" pattern.

reference "proxy", that calls real object, if object is disposed (replaced by new one). The proxy will try to call new one or look for the same object with same Id.

[Flavelius]

The managed objects are, in a way, already proxies. Afaik they mainly rely on an object handle to call into the native side. But the problem is that with a new script the type of the object changes (and with that its members etc.), so proxying with that pattern won't work (and actually changing the type of an object to an unrelated one is not possible in c#/.net as far as i know).

But it could make sense to have a SetScript<T> method that returns a reference to the new object.

[Mupli]

But it could make sense to have a SetScript method that returns a reference to the new object.

Well assigning script to node is doing the same thing. So returning new object wont work with [Export].
Type will be still limited to the one that you use. For example for "Panel" you need script that extend "Panel". So the Proxy should use same Type for original and for proxy. But we should have feature utils to Recast a proxy to proper type.