godot: C# Script Registration that use Generics can error out in the ScriptManagerBridge

Godot version

4.1.stable.mono

System information

Godot v4.1.stable.mono - Windows 10.0.22621 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 (NVIDIA; 31.0.15.3640) - AMD Ryzen 7 5800X 8-Core Processor (16 Threads)

Issue description

This issue occurs when two scripts are derived from a generic script that uses the same type.

Say you have a script, GenericScript<T>:

using Godot;
using System.Collections.Generic;

public partial class GenericScript<T> : Node3D
{
  protected List<T> someList = new List<T>();
}

And we have two scripts that derive from that class with the same type parameter:

public partial class TestScript : GenericScript<float>
{
	// Called when the node enters the scene tree for the first time.
	public override void _Ready()
	{
		someList.Add(1);
	}
}

public partial class OtherTestScript : GenericScript<float>
{

}

Building the mono project while a scene is loaded will present an error. See the reproduction steps below.

Steps to reproduce

Step 1

Create a scene that has two nodes. One node has TestScript attached, the other node has OtherTestScript attached (see above for definition).

Step 2

Rebuild Mono from the MSBuild tab in the bottom right.

You will see the following error in the console:

modules/mono/glue/runtime_interop.cpp:1324 - System.ArgumentException: An item with the same key has already been added. Key: GenericScript`1[System.Single]
     at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
     at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
     at Godot.Bridge.ScriptManagerBridge.ScriptTypeBiMap.Add(IntPtr scriptPtr, Type scriptType) in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs:line 23
     at Godot.Bridge.ScriptManagerBridge.TryReloadRegisteredScriptWithClass(IntPtr scriptPtr) in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs:line 579

Step 3A

If you Rebuild Mono again, you will see the following error and Godot will no longer be in a good state.

modules/mono/mono_gd/gd_mono.cpp:513 - .NET: Failed to unload assemblies. Please check https://github.com/godotengine/godot/issues/78513 for more information. (User)

Step 3B

If you instead close the scene so that no scenes are loaded and Rebuild Mono, the rebuild will appear successful and no errors will be reported.

Minimal reproduction project

A repro project can be found in this repository. At the time of writing this ticket, the latest version was 5f6983c.

The repro contains the scripts defined above, a test scene, and a README.md file with steps essentially the same as the reproduction steps above.

About this issue

Original URL
State: closed
Created a year ago
Reactions: 25
Comments: 22 (3 by maintainers)

Links to this issue

Commits related to this issue

[Fix] ScriptManagerBridge duplicate key Type This PR is related to fix #79519. ## Summary When the same key is added in `ScriptManagerBridge`, throws an Exception and breaks Unload. Here is ... — committed to ricaun/godot by ricaun 9 months ago
Added singleton regions And found issue with the Singleton base class: https://github.com/godotengine/godot/issues/79519 — committed to davidmenard0/openmoba by davidmenard0 6 months ago

Most upvoted comments

This is a good write up. I created a similar repro here (though I didn’t make the connection between matching types) in a related issue but this should be its own thing or even supersede the previous issue if there’s no other causes.

If it ends up being that using generic scripts is simply not supported, I would suggest that this is added to the documentation and a warning error is produced upon contact with such a script.

I really hope this isn’t the case, I would despair.

Edit: I think the issue lies in why the classnames aren’t being used in the dictionary instead of the base class. Some kind of compiler reload weirdness I would expect.

dannymate on Jul 15, 2023

Hello,

I rebuilt the GodotSharp and basically changed Add to TryAdd. https://github.com/ricaun/godot/blob/09191b1bed27adc574cb444adf2c8781a7585fd0/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs#L24-L26

The message Missing class qualified name for reloading script happens sometimes, still need to figure out this part, but is normal because I’m ignoring some pointers that have the same type.

In the end, the reload looks like is working fine.

Here is my custom build using the 4.1.1-stable: https://github.com/ricaun/godot/releases/tag/4.1.1-stable-fix-generic-class

ricaun on Sep 25, 2023

Is there a solution for this problem I am currently running into this issue in v4.2.1.stable.mono.official.b09f793f5

This issue should be fixed in 4.3, you can try 4.3 dev4 out and see if it is fixed on your use case.

Delsin-Yu on Mar 11, 2024

#87550 should fix this issue. After deeper debugging, we believe this issue is caused by an incorrect script reloading order, that is and I quote from the PR:

After successfully performing an Assembly Unloading, it is crucial for the Native side to reload the list of unloaded scripts in a specific order.

For example: a ChildClass must have its BaseClass reloaded before getting passed into the C# reloading method, otherwise, since after unloading, there is no way for the C# side to recognize this new BaseClass, it will create and tie a new instance of BaseClass back to the Native side, this will eventually causing Duplicate Key Exception when the actual pending BaseClass is reloading.

CSharpScriptDepSort contains the algorithm for sorting these reload-pending scripts, this method relies on CSharpScript::get_base_script() for obtaining the base script of a CSharp script. Unfortunately, for generic inheritances, the base type for a GenericChildClass : GenericBaseClass<int> is not GenericBaseClass<T> which we have a path for, instead the actual base type is GenericBaseClass<int> that never has a script_path, so the get_base_script() method always returns null for GenericChildClass in this case, hence it breaks the sorting method, causing unpredictable script reloading order, and we eventually get Duplicate Key Exception on the Managed side if that random order happens to be incorrect.

The PR adds a new get_base_script dedicated to this sorting algorithm, which has everything the original algorithm does, except it replaces the script_path empty checking with class_name empty checking, this should fix the script ordering issue.

However, GenericBaseClass<int> is a valid type, so the public API get_base_script should return the actual CSharpScript instance, instead of a null value, but assigning path to these in-between types proves challenging, we cannot duplicate the script_path property of the original script GenericBaseClass<T> since Godot resources don't allow multiple resources sharing the same path, so a separate issue and/or pr is required on implementing support for some sorts of virtual paths, such as res://path/to/GenericBaseClass.cs: Int32, but that is out of the scope of the PR.

Delsin-Yu on Jan 25, 2024

I had this problem. I tried 4.3 dev5 and it seems gone (I had a error message at first but it disappeared and I didn’t save it and wasn’t able to make it appears again, sorry).

I just pulled down 4.3 dev5 in an attempt to fix this issue and got this error on the first load (haven’t had errors since). Is this the same one you saw?

Assertion failed: Script path can't be empty.
    Details: 
     at Godot.GodotTraceListener.Fail(String message, String detailMessage) in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotTraceListener.cs:line 24
     at System.Diagnostics.TraceInternal.Fail(String message, String detailMessage)
     at System.Diagnostics.Debug.Fail(String message, String detailMessage)
     at Godot.Bridge.ScriptManagerBridge.GetGlobalClassName(godot_string* scriptPath, godot_string* outBaseType, godot_string* outIconPath, godot_string* outClassName) in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs:line 232

maxloy on Apr 16, 2024

It turns out the issue is related to Generic Derivatives holding empty Script Path on the native side, which requires further investigation, the approach I took seems like a workaround that just happens to suppress this issue: It makes ReflectionUtils.FindTypeInLoadedAssemblies returns null forever, and causing early returns on the caller site which prevents the new type to entering BiMap at all, and in our cause, only those duplicated generic derivatives are ever went down here.

~~The issue appears to be ReflectionUtils.FindTypeInLoadedAssemblies, current implementation returns a different instance of Type when the typeFullName is a generic type, weird.~~

AppDomain.CurrentDomain.GetAssemblies() .FirstOrDefault(a => a.GetName().Name == assemblyName)? .GetType(typeFullName)

This new approach appears to return the same instance, so it should fix related issues.

Type.GetType($"{assemblyName}.{typeFullName}")

Fix validated using This Repo, and our in-house project (which is extra complex and deeply suffers from this issue).

Delsin-Yu on Dec 30, 2023

Have the same problem,

I am going to try to recompile the Bridge and implement something to ignore repeated Type.

Like this code: https://github.com/godotengine/godot/blob/bd6af8e0ea69167dd0627f3bd54f9105bda0f8b5/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs#L68-L70

ricaun on Sep 25, 2023

Hello,

I rebuilt the GodotSharp and basically changed Add to TryAdd. https://github.com/ricaun/godot/blob/09191b1bed27adc574cb444adf2c8781a7585fd0/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs#L24-L26

The message Missing class qualified name for reloading script happens sometimes, still need to figure out this part, but is normal because I’m ignoring some pointers that have the same type.

In the end, the reload looks like is working fine.

Here is my custom build using the 4.1.1-stable: https://github.com/ricaun/godot/releases/tag/4.1.1-stable-fix-generic-class

I cannot say if this comes with any side-effect, but I prefer Godot remaining in a stable state with it complaining of missing class qualified name than it breaking down and requiring a restart every 2 build. So, for the moment, you are my hero.

I’ve been having the same issue for the past few days and stumbled onto this thread from my investigations. Your build seem to do the trick for now. Hopefully a more stable solution will be found.

CantyCanadian on Oct 1, 2023