Make PrototypeManager.TryIndex log errors when using invalid id str…

…ucts (#5203) * Make `PrototypeManager.TryIndex` log errors when using id structs * A
space-wizards · Jun 8, 2024 · 94f9807 · 94f9807
1 parent 5aa9378
commit 94f9807
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 19 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -39,7 +39,7 @@ END TEMPLATE-->
 
 ### New features
 
-*None yet*
+* `IPrototypeManager.TryIndex` will now default to logging errors if passed an invalid prototype id struct (i,e., `EntProtoId` or `ProtoId<T>`). There is a new optional bool argument to disable logging errors.
 
 ### Bugfixes
 

diff --git a/Robust.Shared/Prototypes/IPrototypeManager.cs b/Robust.Shared/Prototypes/IPrototypeManager.cs
@@ -143,17 +143,20 @@ bool TryGetInstances<T>([NotNullWhen(true)] out FrozenDictionary<string, T>? ins
     /// </summary>
     FrozenDictionary<string, T> GetInstances<T>() where T : IPrototype;
 
-    /// <inheritdoc cref="TryIndex{T}(string, out T)"/>
-    bool TryIndex(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype);
+    /// <inheritdoc cref="TryIndex{T}(ProtoId{T}, out T, bool)"/>
+    bool TryIndex(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype,  bool logError = true);
 
-    /// <inheritdoc cref="TryIndex{T}(string, out T)"/>
-    bool TryIndex<T>(ProtoId<T> id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype;
+    /// <summary>
+    /// Attempt to retrieve the prototype corresponding to the given prototype id.
+    /// Unless otherwise specified, this will log an error if the id does not match any known prototype.
+    /// </summary>
+    bool TryIndex<T>(ProtoId<T> id, [NotNullWhen(true)] out T? prototype,  bool logError = true) where T : class, IPrototype;
 
-    /// <inheritdoc cref="TryIndex{T}(string, out T)"/>
-    bool TryIndex(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype);
+    /// <inheritdoc cref="TryIndex{T}(ProtoId{T}, out T, bool)"/>
+    bool TryIndex(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype,  bool logError = true);
 
-    /// <inheritdoc cref="TryIndex{T}(string, out T)"/>
-    bool TryIndex<T>(ProtoId<T>? id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype;
+    /// <inheritdoc cref="TryIndex{T}(ProtoId{T}, out T, bool)"/>
+    bool TryIndex<T>(ProtoId<T>? id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype;
 
     bool HasMapping<T>(string id);
     bool TryGetMapping(Type kind, string id, [NotNullWhen(true)] out MappingDataNode? mappings);

diff --git a/Robust.Shared/Prototypes/PrototypeManager.Categories.cs b/Robust.Shared/Prototypes/PrototypeManager.Categories.cs
@@ -108,7 +108,7 @@ private IReadOnlySet<EntityCategoryPrototype> UpdateCategories(EntProtoId id,
             }
         }
 
-        DebugTools.Assert(!TryIndex(id, out var instance)
+        DebugTools.Assert(!TryIndex(id, out var instance, false)
                           || instance.CategoriesInternal == null
                           || instance.CategoriesInternal.All(x =>
                               set.Any(y => y.ID == x)));
@@ -124,7 +124,7 @@ private IReadOnlySet<EntityCategoryPrototype> UpdateCategories(EntProtoId id,
             }
         }
 
-        if (!TryIndex(id, out var protoInstance))
+        if (!TryIndex(id, out var protoInstance, false))
         {
             // Prototype is abstract
             cache.Add(id, set);

diff --git a/Robust.Shared/Prototypes/PrototypeManager.cs b/Robust.Shared/Prototypes/PrototypeManager.cs
@@ -744,39 +744,49 @@ public bool TryIndex(Type kind, string id, [NotNullWhen(true)] out IPrototype? p
         }
 
         /// <inheritdoc />
-        public bool TryIndex(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype)
+        public bool TryIndex(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype, bool logError = true)
         {
-            return TryIndex(id.Id, out prototype);
+            if (TryIndex(id.Id, out prototype))
+                return true;
+
+            if (logError)
+                Sawmill.Error($"Attempted to resolve invalid {nameof(EntProtoId)}: {id.Id}");
+            return false;
         }
 
         /// <inheritdoc />
-        public bool TryIndex<T>(ProtoId<T> id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype
+        public bool TryIndex<T>(ProtoId<T> id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype
         {
-            return TryIndex(id.Id, out prototype);
+            if (TryIndex(id.Id, out prototype))
+                return true;
+
+            if (logError)
+                Sawmill.Error($"Attempted to resolve invalid ProtoId<{typeof(T).Name}>: {id.Id}");
+            return false;
         }
 
         /// <inheritdoc />
-        public bool TryIndex(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype)
+        public bool TryIndex(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype, bool logError = true)
         {
             if (id == null)
             {
                 prototype = null;
                 return false;
             }
 
-            return TryIndex(id.Value, out prototype);
+            return TryIndex(id.Value, out prototype, logError);
         }
 
         /// <inheritdoc />
-        public bool TryIndex<T>(ProtoId<T>? id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype
+        public bool TryIndex<T>(ProtoId<T>? id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype
         {
             if (id == null)
             {
                 prototype = null;
                 return false;
             }
 
-            return TryIndex(id.Value, out prototype);
+            return TryIndex(id.Value, out prototype, logError);
         }
 
         /// <inheritdoc />