Fix C# ScriptManagerBridge duplicate key Type #83204
Conversation
This PR is related to fix godotengine#79519. ## Summary When the same key is added in `ScriptManagerBridge`, throws an Exception and breaks Unload. Here is a minimal project example with the issue: https://github.com/taylorhadden/godot-csharp-generic-assembly-reload-error This PR only adds the key if not exist, fixing the problem but sometimes the `Missing class qualified name for reloading script` happens. https://github.com/godotengine/godot/blob/b1371806ad3907c009458ea939bd4b810f9deb21/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs#L558 I didn't figure out how to prevent that message Error.
AThousandShips
changed the title
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs
Outdated
Show resolved
Hide resolved
Remove whitespace
| // Due to generic classes, more than one class can point to the same type, so | ||
| // there could be duplicate keys in this case. We only add a type as key once. | ||
| _typeScriptMap.TryAdd(scriptType, scriptPtr); |
There was a problem hiding this comment.
This doesn't sound right, generic classes can't be scripts so they shouldn't be added to the map. If a generic type reaches this point, then we have a bug somewhere else where we should be filtering/ignoring generic classes.
There was a problem hiding this comment.
You are right, filtering the types would be the best approach. I didn't find a way to do that. 😐
I you have any idea when/where the types are created, that would help.
There was a problem hiding this comment.
I think you need to update the other entry as well (removing the old script pointer). Based on my test the change I propose can reduce the issue into an Editor Error Message instead of Crashing the Mono module (Fail to unload assembly on second build). However, I agree that finding the root cause is necessary.
public void Add(IntPtr scriptPtr, Type scriptType)
{
// TODO: What if this is called while unloading a load context, but after we already did cleanup in preparation for unloading?
if (_typeScriptMap.TryGetValue(scriptType, out var oldPtr))
{
_scriptTypeMap.Remove(oldPtr);
_typeScriptMap[scriptType] = scriptPtr;
}
else
{
_typeScriptMap.Add(scriptType, scriptPtr);
}
_scriptTypeMap.Add(scriptPtr, scriptType);
|
Superseded by #87550 |
Issue
This PR is related to fix #79519.
Summary
When the same key is added in
ScriptManagerBridge, throws an Exception and breaks Unload.Here is a minimal project example with the issue: https://github.com/taylorhadden/godot-csharp-generic-assembly-reload-error
This PR only adds the key if not exist, fixing the problem but sometimes the
Missing class qualified name for reloading scripthappens.godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs
Line 558 in b137180
I didn't figure out how to prevent that message Error.