search

raulsntos / godot-dotnet

MIT License
86 stars 13 forks source link

Prevent dispose of Static StringName #23

Open scgm0 opened 2 months ago

scgm0 commented 2 months ago

Try fix #20 again

takethebait commented 2 months ago

While testing, when quitting a scene I no longer get the unreferenced string errors but I now get a couple of orphaned stringNames:

Orphan StringName: _setup_local_to_scene (static: 0, total: 50)
Orphan StringName: OS (static: 2, total: 3)
Orphan StringName: Node3D (static: 2, total: 3)
Orphan StringName: InputEventMouseMotion (static: 2, total: 52)
Orphan StringName: _get_rid (static: 0, total: 50)
StringName: 5 unclaimed string names at exit.

this is on top of all the InputEventMouseMotion events which I believe to be unrelated to the initial issue.

raulsntos commented 2 months ago

I'm not sure we want to ensure static StringNames are only instantiated once. I feel like this makes StringName more complicated and I'd prefer to keep primitive types like StringName very simple.

If we need a mechanism to avoid creating the same StringName more than once, we may want to implement such mechanism on top of StringName rather than modifying the StringName type itself, but I'm not sure we need it.

Since the motivation behind this PR is to fix https://github.com/raulsntos/godot-dotnet/issues/20, let's try to figure out if there's a simpler way to fix this (I've commented on that issue: https://github.com/raulsntos/godot-dotnet/issues/20#issuecomment-2340986836).

scgm0 commented 2 months ago

I'm not sure we want to ensure static StringNames are only instantiated once. I feel like this makes StringName more complicated and I'd prefer to keep primitive types like StringName very simple.

If we need a mechanism to avoid creating the same StringName more than once, we may want to implement such mechanism on top of StringName rather than modifying the StringName type itself, but I'm not sure we need it.

Since the motivation behind this PR is to fix #20, let's try to figure out if there's a simpler way to fix this (I've commented on that issue: #20 (comment)).

I kept the static StringName instantiated only once in this pr because I did not find how to determine whether the native StringName passed by godot is a static StringName. If the native StringName is static, but the instance created by c# is not static, then the c# instance It will be recycled by gc soon and trigger the recycling of native StringName, eventually causing the static StringName reference in godot to be 0. Is there a way for us to determine whether the native StringName is static? In this way, there will be no problem even if StringName is instantiated repeatedly in c#.

scgm0 commented 2 months ago

It seems to fill the need, but it's not public. . . 图片