diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index aff9028bf..ae5ae44ea 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -36,10 +36,12 @@ END TEMPLATE--> ### Breaking changes * More members in `IntegrationInstance` now enforce that the instance is idle before accessing it. +* `IPrototypeManager.TryIndex` no longer logs errors unless using the overload with an optional parameter. Use `Resolve()` instead if error logging is desired. ### New features * `RobustClientPackaging.WriteClientResources()` and `RobustServerPackaging.WriteServerResources()` now have an overload taking in a set of things to ignore in the content resources directory. +* Added `IPrototypeManager.Resolve()`, which logs an error if the resolved prototype does not exist. This is effectively the previous (but not original) default behavior of `IPrototypeManager.TryIndex`. ### Bugfixes diff --git a/Robust.Shared/Prototypes/IPrototypeManager.cs b/Robust.Shared/Prototypes/IPrototypeManager.cs index 49be0fc80..6e4978fdf 100644 --- a/Robust.Shared/Prototypes/IPrototypeManager.cs +++ b/Robust.Shared/Prototypes/IPrototypeManager.cs @@ -143,21 +143,269 @@ public interface IPrototypeManager /// FrozenDictionary GetInstances() where T : IPrototype; - /// - bool TryIndex([ForbidLiteral] EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype, bool logError = true); + // For obsolete APIs. + // ReSharper disable MethodOverloadWithOptionalParameter /// - /// 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. + /// Resolve an by ID, logging an error if it does not exist. /// - bool TryIndex([ForbidLiteral] ProtoId id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype; + /// + /// + /// This method does not throw if the prototype does not exist, and instead simply logs an error and returns false. + /// + /// + /// This method can be used if an invalid prototype ID indicates a programming error somewhere else, + /// acting as a convenient function to make code more defensive. + /// Ideally such errors should also have some other way of being validated, however (e.g. static field validation). + /// + /// + /// This method should not be used when handling IDs that are expected to be invalid, such as user input. + /// Use instead for those. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool Resolve([ForbidLiteral] EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype); - /// - bool TryIndex([ForbidLiteral] EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype, bool logError = true); + /// + /// Retrieve an by ID, optionally logging an error if it does not exist. + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// If true (default), log an error if the prototype does not exist. + /// True if the prototype exists, false if it does not. + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] + bool TryIndex( + [ForbidLiteral] EntProtoId id, + [NotNullWhen(true)] out EntityPrototype? prototype, + bool logError = true); - /// + /// + /// Resolve an by ID. + /// + /// + /// + /// This method does not throw if the prototype does not exist, and instead simply returns false. + /// + /// + /// It is appropriate to use this method when handling IDs from external sources + /// (user input, old save records, etc.), where it is expected that data may be invalid. + /// + /// + /// It is not necessarily appropriate to use this method if an invalid ID indicates a programming error, + /// such as an invalid ID specified in a YAML prototype. In this scenario, + /// usage of this method should always be combined with proper error logging and other methods of validation + /// (e.g. static field validation). + /// can be used as a convenient helper in this scenario. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool TryIndex([ForbidLiteral] EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype); + + /// + /// Resolve a prototype by ID, logging an error if it does not exist. + /// + /// + /// + /// This method does not throw if the prototype does not exist, and instead simply logs an error and returns false. + /// + /// + /// This method can be used if an invalid prototype ID indicates a programming error somewhere else, + /// acting as a convenient function to make code more defensive. + /// Ideally such errors should also have some other way of being validated, however (e.g. static field validation). + /// + /// + /// This method should not be used when handling IDs that are expected to be invalid, such as user input. + /// Use instead for those. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool Resolve([ForbidLiteral] ProtoId id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype; + + /// + /// Retrieve a prototype by ID, optionally logging an error if it does not exist. + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// If true (default), log an error if the prototype does not exist. + /// True if the prototype exists, false if it does not. + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] + bool TryIndex([ForbidLiteral] ProtoId id, [NotNullWhen(true)] out T? prototype, bool logError = true) + where T : class, IPrototype; + + /// + /// Resolve a prototype by ID. + /// + /// + /// + /// This method does not throw if the prototype does not exist, and instead simply returns false. + /// + /// + /// It is appropriate to use this method when handling IDs from external sources + /// (user input, old save records, etc.), where it is expected that data may be invalid. + /// + /// + /// It is not necessarily appropriate to use this method if an invalid ID indicates a programming error, + /// such as an invalid ID specified in a YAML prototype. In this scenario, + /// usage of this method should always be combined with proper error logging and other methods of validation + /// (e.g. static field validation). + /// can be used as a convenient helper in this scenario. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool TryIndex([ForbidLiteral] ProtoId id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype; + + /// + /// Resolve an by ID, gracefully handling null, + /// and logging an error if it does not exist. + /// + /// + /// + /// This method does not throw if the prototype is invalid, and instead simply logs an error and returns false. + /// No error is reported if the ID is simply null. + /// + /// + /// This method can be used if an invalid prototype ID indicates a programming error somewhere else, + /// acting as a convenient function to make code more defensive. + /// Ideally such errors should also have some other way of being validated, however (e.g. static field validation). + /// + /// + /// This method should not be used when handling IDs that are expected to be invalid, such as user input. + /// Use instead for those. + /// + /// + /// The prototype ID to look up. May be null. + /// + /// The prototype that was resolved, null if was null or did not exist. + /// + /// True if the prototype exists, false if was null, or it does not exist. + /// + bool Resolve([ForbidLiteral] EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype); + + /// + /// Retrieve an by ID, gracefully handling null, + /// and optionally logging an error if it does not exist. + /// + /// + /// + /// No error is logged if is null. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// If true (default), log an error if the prototype does not exist. + /// True if the prototype exists, false if was null, or it does not exist. + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] + bool TryIndex( + [ForbidLiteral] EntProtoId? id, + [NotNullWhen(true)] out EntityPrototype? prototype, + bool logError = true); + + /// + /// Resolve an by ID, gracefully handling null. + /// + /// + /// + /// This method does not throw if the prototype does not exist or is null, and instead simply returns false. + /// + /// + /// It is appropriate to use this method when handling IDs from external sources + /// (user input, old save records, etc.), where it is expected that data may be invalid. + /// + /// + /// It is not necessarily appropriate to use this method if an invalid ID indicates a programming error, + /// such as an invalid ID specified in a YAML prototype. In this scenario, + /// usage of this method should always be combined with proper error logging and other methods of validation + /// (e.g. static field validation). + /// can be used as a convenient helper in this scenario. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool TryIndex([ForbidLiteral] EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype); + + /// + /// Resolve a prototype by ID, gracefully handling null, and logging an error if it does not exist. + /// + /// + /// + /// This method does not throw if the prototype is invalid, and instead simply logs an error and returns false. + /// No error is reported if the ID is simply null. + /// + /// + /// This method can be used if an invalid prototype ID indicates a programming error somewhere else, + /// acting as a convenient function to make code more defensive. + /// Ideally such errors should also have some other way of being validated, however (e.g. static field validation). + /// + /// + /// This method should not be used when handling IDs that are expected to be invalid, such as user input. + /// Use instead for those. + /// + /// + /// The prototype ID to look up. May be null. + /// + /// The prototype that was resolved, null if was null or did not exist. + /// + /// True if the prototype exists, false if was null, or it does not exist. + /// + bool Resolve([ForbidLiteral] ProtoId? id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype; + + /// + /// Retrieve a prototype by ID, gracefully handling null, + /// and optionally logging an error if it does not exist. + /// + /// + /// + /// No error is logged if is null. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// If true (default), log an error if the prototype does not exist. + /// True if the prototype exists, false if was null, or it does not exist. + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] bool TryIndex([ForbidLiteral] ProtoId? id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype; + /// + /// Resolve a prototype by ID, gracefully handling null. + /// + /// + /// + /// This method does not throw if the prototype does not exist or is null, and instead simply returns false. + /// + /// + /// It is appropriate to use this method when handling IDs from external sources + /// (user input, old save records, etc.), where it is expected that data may be invalid. + /// + /// + /// It is not necessarily appropriate to use this method if an invalid ID indicates a programming error, + /// such as an invalid ID specified in a YAML prototype. In this scenario, + /// usage of this method should always be combined with proper error logging and other methods of validation + /// (e.g. static field validation). + /// can be used as a convenient helper in this scenario. + /// + /// + /// The prototype ID to look up. + /// The prototype that was resolved, null if it does not exist. + /// True if the prototype exists, false if it does not. + /// + bool TryIndex([ForbidLiteral] ProtoId? id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype; + + // ReSharper restore MethodOverloadWithOptionalParameter + bool HasMapping(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 index feba747cf..739a8eee3 100644 --- a/Robust.Shared/Prototypes/PrototypeManager.Categories.cs +++ b/Robust.Shared/Prototypes/PrototypeManager.Categories.cs @@ -108,7 +108,7 @@ public abstract partial class PrototypeManager : IPrototypeManagerInternal } } - DebugTools.Assert(!TryIndex(id, out var instance, false) + DebugTools.Assert(!TryIndex(id, out var instance) || instance.CategoriesInternal == null || instance.CategoriesInternal.All(x => set.Any(y => y.ID == x))); @@ -124,7 +124,7 @@ public abstract partial class PrototypeManager : IPrototypeManagerInternal } } - if (!TryIndex(id, out var protoInstance, false)) + if (!TryIndex(id, out var protoInstance)) { // Prototype is abstract cache.Add(id, set); diff --git a/Robust.Shared/Prototypes/PrototypeManager.cs b/Robust.Shared/Prototypes/PrototypeManager.cs index 76d92fb80..dfd9a0c9f 100644 --- a/Robust.Shared/Prototypes/PrototypeManager.cs +++ b/Robust.Shared/Prototypes/PrototypeManager.cs @@ -788,42 +788,76 @@ namespace Robust.Shared.Prototypes return index.Instances.TryGetValue(id, out prototype); } - /// + + // For obsolete APIs. + // ReSharper disable MethodOverloadWithOptionalParameter + + public bool Resolve(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype) + { + if (TryIndex(id.Id, out prototype)) + return true; + + Sawmill.Error($"Attempted to resolve invalid {nameof(EntProtoId)}: {id.Id}\n{Environment.StackTrace}"); + return false; + } + + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] public bool TryIndex(EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype, bool logError = true) { - if (TryIndex(id.Id, out prototype)) - return true; - if (logError) - Sawmill.Error($"Attempted to resolve invalid {nameof(EntProtoId)}: {id.Id}"); - return false; + return Resolve(id, out prototype); + return TryIndex(id, out prototype); } - /// - public bool TryIndex(ProtoId id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype + public bool TryIndex([ForbidLiteral] EntProtoId id, [NotNullWhen(true)] out EntityPrototype? prototype) + { + return TryIndex(id.Id, out prototype); + } + + public bool Resolve(ProtoId id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype { if (TryIndex(id.Id, out prototype)) return true; - if (logError) - Sawmill.Error($"Attempted to resolve invalid ProtoId<{typeof(T).Name}>: {id.Id}"); + Sawmill.Error($"Attempted to resolve invalid ProtoId<{typeof(T).Name}>: {id.Id}\n{Environment.StackTrace}"); return false; } - /// + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] + public bool TryIndex(ProtoId id, [NotNullWhen(true)] out T? prototype, bool logError = true) + where T : class, IPrototype + { + if (logError) + return Resolve(id, out prototype); + return TryIndex(id, out prototype); + } + + public bool TryIndex(ProtoId id, [NotNullWhen(true)] out T? prototype) + where T : class, IPrototype + { + return TryIndex(id.Id, out prototype); + } + + public bool Resolve(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype) + { + if (id == null) + { + prototype = null; + return false; + } + + return Resolve(id.Value, out prototype); + } + + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] 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, logError); + if (logError) + return Resolve(id, out prototype); + return TryIndex(id, out prototype); } - /// - public bool TryIndex(ProtoId? id, [NotNullWhen(true)] out T? prototype, bool logError = true) where T : class, IPrototype + public bool TryIndex(EntProtoId? id, [NotNullWhen(true)] out EntityPrototype? prototype) { if (id == null) { @@ -831,9 +865,43 @@ namespace Robust.Shared.Prototypes return false; } - return TryIndex(id.Value, out prototype, logError); + return TryIndex(id.Value, out prototype); } + public bool Resolve(ProtoId? id, [NotNullWhen(true)] out T? prototype) where T : class, IPrototype + { + if (id == null) + { + prototype = null; + return false; + } + + return Resolve(id.Value, out prototype); + } + + [Obsolete("Use Resolve() if you want to get a prototype without throwing but while still logging an error.")] + public bool TryIndex(ProtoId? id, [NotNullWhen(true)] out T? prototype, bool logError = true) + where T : class, IPrototype + { + if (logError) + return Resolve(id, out prototype); + return TryIndex(id, out prototype); + } + + public bool TryIndex(ProtoId? id, [NotNullWhen(true)] out T? prototype) + where T : class, IPrototype + { + if (id == null) + { + prototype = null; + return false; + } + + return TryIndex(id.Value, out prototype); + } + + // ReSharper restore MethodOverloadWithOptionalParameter + /// public bool HasMapping(string id) {