space-wizards / RobustToolbox

Robust multiplayer game engine, used by Space Station 14
https://spacestation14.io
Other
498 stars 377 forks source link

enum reflection needs to do a better job of matching the name and not just the suffix #2280

Open xRiriq opened 2 years ago

xRiriq commented 2 years ago

I ran into this situation while working on space-station-14:

namespace Content.Shared.MedicalScanner
{
    [Serializable, NetSerializable]
    public enum HandheldMedicalScannerUiKey
    {
        Key,
    }
    [Serializable, NetSerializable]
    public enum MedicalScannerUiKey
    {
    Key
    }
}

What ends up happening in this situation based in the reflection manager is that we end up matching the suffix, and because of incidental or lexical order the first enum is selected instead of the second. This can be seen below:

foreach (var type in assembly.DefinedTypes)
{
    if (!type.IsEnum || !type.FullName!.EndsWith(typeName))
    {
    continue;
    }

    @enum = (Enum) Enum.Parse(type, value);
    _enumCache[reference] = @enum;
    return true;
}

https://github.com/space-wizards/RobustToolbox/blame/0fd210481ac67fc8e17cf7475e196ae646f5764b/Robust.Shared/Reflection/ReflectionManager.cs#L197-L200

The only way I could detect this problem was in frustration changing the name of the first key:

namespace Content.Shared.MedicalScanner
{
    [Serializable, NetSerializable]
    public enum HandheldMedicalScannerUiKey
    {
        HandheldMedicalScannerKey,
    }
    [Serializable, NetSerializable]
    public enum MedicalScannerUiKey
    {
    Key
    }
}

The stacktrace then becomes the following which is why I was able to see this error clearly.

[FATL] unhandled: System.ArgumentException: Requested value 'Key' was not found.
   at System.Enum.TryParseByName(RuntimeType enumType, ReadOnlySpan`1 value, Boolean ignoreCase, Boolean throwOnFailure, UInt64& result)
   at System.Enum.TryParseInt32Enum(RuntimeType enumType, ReadOnlySpan`1 value, Int32 minInclusive, Int32 maxInclusive, Boolean ignoreCase, Boolean throwOnFailure, TypeCode type, Int32& result)
   at System.Enum.TryParse(Type enumType, ReadOnlySpan`1 value, Boolean ignoreCase, Boolean throwOnFailure, Object& result)
   at System.Enum.Parse(Type enumType, String value)
   at Robust.Shared.Reflection.ReflectionManager.TryParseEnumReference(String reference, Enum& enum) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Reflection\ReflectionManager.cs:line 202
   at Robust.Shared.GameObjects.SharedUserInterfaceComponent.PrototypeData.Robust.Shared.Serialization.ISerializationHooks.AfterDeserialization() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\GameObjects\Components\UserInterface\SharedUserInterfaceComponent.cs:line 31
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 56
   at Robust.Shared.Serialization.Manager.Result.DeserializedCollection`2.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedCollection.cs:line 69
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 52
   at Robust.Shared.Serialization.Manager.Result.DeserializedComponentRegistry.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedComponentRegistry.cs:line 107
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 52
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 435
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.Resync() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 382
   at Robust.Client.GameController.StartupContinue(DisplayMode displayMode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.cs:line 131
   at Robust.Client.GameController.ContinueStartupAndLoop(DisplayMode mode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 122
   at Robust.Client.GameController.GameThreadMain(DisplayMode mode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 114
   at Robust.Client.GameController.<>c__DisplayClass80_0.<Run>b__0() in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 81
   at System.Threading.Thread.StartCallback()
Unhandled exception. System.ArgumentException: Requested value 'Key' was not found.
   at System.Enum.TryParseByName(RuntimeType enumType, ReadOnlySpan`1 value, Boolean ignoreCase, Boolean throwOnFailure, UInt64& result)
   at System.Enum.TryParseInt32Enum(RuntimeType enumType, ReadOnlySpan`1 value, Int32 minInclusive, Int32 maxInclusive, Boolean ignoreCase, Boolean throwOnFailure, TypeCode type, Int32& result)
   at System.Enum.TryParse(Type enumType, ReadOnlySpan`1 value, Boolean ignoreCase, Boolean throwOnFailure, Object& result)
   at System.Enum.Parse(Type enumType, String value)
   at Robust.Shared.Reflection.ReflectionManager.TryParseEnumReference(String reference, Enum& enum) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Reflection\ReflectionManager.cs:line 202
   at Robust.Shared.GameObjects.SharedUserInterfaceComponent.PrototypeData.Robust.Shared.Serialization.ISerializationHooks.AfterDeserialization() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\GameObjects\Components\UserInterface\SharedUserInterfaceComponent.cs:line 31
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 56
   at Robust.Shared.Serialization.Manager.Result.DeserializedCollection`2.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedCollection.cs:line 69
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 52
   at Robust.Shared.Serialization.Manager.Result.DeserializedComponentRegistry.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedComponentRegistry.cs:line 107
   at Robust.Shared.Serialization.Manager.Result.DeserializedDefinition`1.CallAfterDeserializationHook() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Serialization\Manager\Result\DeserializedDefinition.cs:line 52
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 435
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.PushInheritance(Type type, String id, DeserializationResult baseResult, HashSet`1 changed) in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 425
   at Robust.Shared.Prototypes.PrototypeManager.Resync() in D:\projects\space-station-14\RobustToolbox\Robust.Shared\Prototypes\PrototypeManager.cs:line 382
   at Robust.Client.GameController.StartupContinue(DisplayMode displayMode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.cs:line 131
   at Robust.Client.GameController.ContinueStartupAndLoop(DisplayMode mode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 122
   at Robust.Client.GameController.GameThreadMain(DisplayMode mode) in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 114
   at Robust.Client.GameController.<>c__DisplayClass80_0.<Run>b__0() in D:\projects\space-station-14\RobustToolbox\Robust.Client\GameController\GameController.Standalone.cs:line 81
   at System.Threading.Thread.StartCallback()
xRiriq commented 2 years ago

This has so many potential side effects I'm not comfortable making a change here without more experience, but it ends up being very hard to detect.