From 4851e913b0a3fa03cf5e3a76c6849312d35543bf Mon Sep 17 00:00:00 2001 From: Errant <35878406+Errant-4@users.noreply.github.com> Date: Fri, 11 Jul 2025 20:37:39 +0200 Subject: [PATCH] More TimespanSerializer improvements (#5910) * improved public TryTimeSpan * don't want any locale shenanigans or misconceptions with the input * missed a test line * also support capitalized time unit indicators * Doesn't need to be nullable. --------- Co-authored-by: PJB3005 --- .../Implementations/TimespanSerializer.cs | 52 ++------------ Robust.Shared/Utility/TimeSpanExt.cs | 68 +++++++++++++++++++ 2 files changed, 72 insertions(+), 48 deletions(-) diff --git a/Robust.Shared/Serialization/TypeSerializers/Implementations/TimespanSerializer.cs b/Robust.Shared/Serialization/TypeSerializers/Implementations/TimespanSerializer.cs index ea2dd799ac..ca7675ba9b 100644 --- a/Robust.Shared/Serialization/TypeSerializers/Implementations/TimespanSerializer.cs +++ b/Robust.Shared/Serialization/TypeSerializers/Implementations/TimespanSerializer.cs @@ -1,5 +1,4 @@ using System; -using System.Diagnostics.CodeAnalysis; using System.Globalization; using JetBrains.Annotations; using Robust.Shared.IoC; @@ -24,11 +23,10 @@ public sealed class TimespanSerializer : ITypeSerializer? instanceProvider = null) { - if (TryTimeSpan(node, out var time)) - return time.Value; + if (TimeSpanExt.TryTimeSpan(node, out var time)) + return time; - var seconds = double.Parse(node.Value, CultureInfo.InvariantCulture); - return TimeSpan.FromSeconds(seconds); + throw new FormatException($"The input string '{node.Value }' can't be converted to TimeSpan"); } public ValidationNode Validate( @@ -37,7 +35,7 @@ public sealed class TimespanSerializer : ITypeSerializer - /// Convert strings from the compatible format (or just numbers) into TimeSpan and output it. Returns true if successful. - /// The string must start with a number and end with a single letter referring to the time unit used. - /// It can NOT combine multiple types (like "1h30m"), but it CAN use decimals ("1.5h") - /// - private bool TryTimeSpan(ValueDataNode node, [NotNullWhen(true)] out TimeSpan? timeSpan) - { - timeSpan = null; - - // A lot of the checks will be for plain numbers, so might as well rule them out right away, instead of - // running all the other checks on them. They will need to get parsed later anyway, if they weren't now. - if (Parse.TryDouble(node.Value, out var v)) - { - timeSpan = TimeSpan.FromSeconds(v); - return true; - } - - // If there aren't even enough characters for a number and a time unit, exit - if (node.Value.Length <= 1) - return false; - - // If the input without the last character is still not a valid number, exit - if (!Parse.TryDouble(node.Value.AsSpan()[..^1], out var number)) - return false; - - // Check the last character of the input for time unit indicators - switch (node.Value[^1]) - { - case 's': - timeSpan = TimeSpan.FromSeconds(number); - return true; - case 'm': - timeSpan = TimeSpan.FromMinutes(number); - return true; - case 'h': - timeSpan = TimeSpan.FromHours(number); - return true; - default: - return false; - } - } } diff --git a/Robust.Shared/Utility/TimeSpanExt.cs b/Robust.Shared/Utility/TimeSpanExt.cs index 41211cd940..33843e7346 100644 --- a/Robust.Shared/Utility/TimeSpanExt.cs +++ b/Robust.Shared/Utility/TimeSpanExt.cs @@ -1,4 +1,7 @@ using System; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using Robust.Shared.Serialization.Markdown.Value; namespace Robust.Shared.Utility; @@ -18,4 +21,69 @@ public static class TimeSpanExt { return TimeSpan.FromTicks(time.Ticks * factor); } + + /// + /// Validates if the input can be turned into a TimeSpan, and outputs it. + /// + /// The data node being validated. It must be either a number (which will be interpreted as seconds) or formatted as a brief alphanumeric timespan. + /// A valid brief alphanumeric timespan starts with a number and ends with a single letter indicating the time unit used. + /// It can NOT combine multiple types (like "1h30m"), but it CAN use decimals ("1.5h") + /// The TimeSpan result. + /// Returns true if the input could be resolved as a TimeSpan.> + public static bool TryTimeSpan(ValueDataNode node, out TimeSpan timeSpan) + { + return TryTimeSpan(node.Value, out timeSpan); + } + + /// + /// Validates if the input can be turned into a TimeSpan, and outputs it. + /// + /// The string being validated. It must be either a number (which will be interpreted as seconds) or formatted as a brief alphanumeric timespan. + /// A valid brief alphanumeric timespan starts with a number and ends with a single letter indicating the time unit used. + /// It can NOT combine multiple types (like "1h30m"), but it CAN use decimals ("1.5h") + /// The TimeSpan result. + /// Returns true if the input could be resolved as a TimeSpan.> + public static bool TryTimeSpan(string str, out TimeSpan timeSpan) + { + timeSpan = TimeSpan.Zero; + + // If someone tried to use comma as a decimal separator, they would get orders of magnitude higher numbers than intended + if (str.Contains(',') || str.Contains(' ') || str.Contains(':')) + return false; + + // A lot of the checks will be for plain numbers, so might as well rule them out right away, instead of + // running all the other checks on them. They will need to get parsed later anyway, if they weren't now. + if (double.TryParse(str, CultureInfo.InvariantCulture, out var v)) + { + timeSpan = TimeSpan.FromSeconds(v); + return true; + } + + // If there aren't even enough characters for a number and a time unit, exit + if (str.Length <= 1) + return false; + + // If the input without the last character is still not a valid number, exit + if (!double.TryParse(str.AsSpan()[..^1], CultureInfo.InvariantCulture, out var number)) + return false; + + // Check the last character of the input for time unit indicators + switch (str[^1]) + { + case 's': + case 'S': + timeSpan = TimeSpan.FromSeconds(number); + return true; + case 'm': + case 'M': + timeSpan = TimeSpan.FromMinutes(number); + return true; + case 'h': + case 'H': + timeSpan = TimeSpan.FromHours(number); + return true; + default: + return false; + } + } }