From 459674ce3b17b9d710bdfa05f4f24065c479e942 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:20:22 -0700 Subject: [PATCH] fix(commands): parse and edit IPoint2D/IPoint3D properties (#2646) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Symptom `[set TargetLocation (x, y)` (quoted or not) answers **"That is not properly formatted."**, and in `[props` the `>` next to `TargetLocation` does nothing when the value is null — which is its normal idle state (`BaseAI.cs:650` clears it). This looked like a `Point3D` parsing regression from #2624/#2625, but `Point3D`/`Point2D`-typed properties (`Location`, etc.) were never affected. The only `[CommandProperty]` in the tree declared as an **interface** is `BaseCreature.TargetLocation : IPoint2D` (`BaseCreature.cs:1111`), and both code paths only knew the structs. `git log -S"IPoint"` over the parser and gump files hits nothing but the initial import — the gap is inherited from RunUO, not recent. ## Root cause - **`[set`** — `Types.TryParse` has no branch for `IPoint2D`/`IPoint3D`. An interface has no static `Parse`, so `GetParseMethod` returns null and the value falls into `Convert.ChangeType("(x, y)", typeof(IPoint2D))`, which throws → "not properly formatted". - **Props gump** — `PropsGump` routes on `obj?.GetType() ?? prop.PropertyType` (since #2180). With a null value the type is `IPoint2D`; `Point2D.IsAssignableFrom(IPoint2D)` is false, no branch matches, and the click is inert. It only worked when the slot already held a `Point2D`, because the runtime type is then the struct. ## Fix - `Types.TryParse`: `IPoint3D`/`IPoint2D` targets resolve to the concrete struct — `Point3D` first, then `Point2D` for an `IPoint2D` target (a 3-tuple is a valid `IPoint2D`). `(-null-)` still clears; the existing null branch runs first. - `PropsGump`: the interface types route to `SetPoint3DGump`/`SetPoint2DGump`. The entity branch stays ahead of them — `TargetLocation` legitimately holds a Mobile too (`ShepherdsCrook.cs:148`, herding toward the shepherd), and that case still opens `SetObjectGump`. - `SetPoint2DGump`/`SetPoint3DGump`: seed the text entries from `value is IPoint2D/IPoint3D` rather than a hard cast, so a `Point3D` sitting in an `IPoint2D` slot cannot `InvalidCast`. ## Not covered `[set TargetLocation 0x40001234` (assigning a mobile by serial) still reports "not properly formatted" — the entity branch keys on the *target* type being `IEntity`, which `IPoint2D` isn't. Real state, but niche; left out to keep this to the reported symptom. ## Testing Five cases in `InterfacePointParseTests`, watched fail before the change (three returned the error string; two pin existing behaviour that must survive): tuple → `Point3D` for both interfaces, pair → `Point2D`, pair rejected for `IPoint3D`, `(-null-)` clears. `dotnet build` 0 warnings. **1059 UOContent** and **891 Server** tests pass, 0 failures. The gump routing is a one-line branch with no automated test — needs an in-game check: `[props` a creature with a null `TargetLocation`, press `>`, expect the Point2D editor. --- .../Tests/Utilities/TryParseTests.cs | 35 +++++++++++++++++++ Projects/UOContent/Gumps/Props/PropsGump.cs | 6 ++-- .../UOContent/Gumps/Props/SetPoint2DGump.cs | 2 +- .../UOContent/Gumps/Props/SetPoint3DGump.cs | 2 +- Projects/UOContent/Utilities/Types.cs | 19 ++++++++++ 5 files changed, 60 insertions(+), 4 deletions(-) diff --git a/Projects/UOContent.Tests/Tests/Utilities/TryParseTests.cs b/Projects/UOContent.Tests/Tests/Utilities/TryParseTests.cs index 314290264..b6c693c84 100644 --- a/Projects/UOContent.Tests/Tests/Utilities/TryParseTests.cs +++ b/Projects/UOContent.Tests/Tests/Utilities/TryParseTests.cs @@ -45,3 +45,38 @@ public class TryParseTests } } } + +// Interface-typed properties (BaseCreature.TargetLocation : IPoint2D) have no static Parse, so +// without these branches the parser fell through to Convert.ChangeType and reported the value as +// "not properly formatted". +public class InterfacePointParseTests +{ + [Theory] + [InlineData(typeof(IPoint3D), "(1, 2, 3)", 1, 2, 3)] + [InlineData(typeof(IPoint2D), "(1, 2, 3)", 1, 2, 3)] // a 3-tuple is a valid IPoint2D + public void TuplesParseIntoPoint3D(Type type, string value, int x, int y, int z) + { + Assert.Null(Server.Types.TryParse(type, value, out var constructed)); + Assert.Equal(new Point3D(x, y, z), constructed); + } + + [Fact] + public void PairParsesIntoPoint2D() + { + Assert.Null(Server.Types.TryParse(typeof(IPoint2D), "(4, 5)", out var constructed)); + Assert.Equal(new Point2D(4, 5), constructed); + } + + [Fact] + public void PairIsNotAPoint3D() + { + Assert.NotNull(Server.Types.TryParse(typeof(IPoint3D), "(4, 5)", out _)); + } + + [Fact] + public void NullSentinelClearsAnInterfaceProperty() + { + Assert.Null(Server.Types.TryParse(typeof(IPoint2D), "(-null-)", out var constructed)); + Assert.Null(constructed); + } +} diff --git a/Projects/UOContent/Gumps/Props/PropsGump.cs b/Projects/UOContent/Gumps/Props/PropsGump.cs index 00fd2557d..e24b2f571 100644 --- a/Projects/UOContent/Gumps/Props/PropsGump.cs +++ b/Projects/UOContent/Gumps/Props/PropsGump.cs @@ -224,11 +224,13 @@ namespace Server.Gumps { from.Target = new SetObjectTarget(prop, from, m_Object, type, this); } - else if (IsType(type, OfPoint3D)) + // A null interface-typed value (TargetLocation : IPoint2D) routes on the + // declared type; a held entity was already caught above. + else if (IsType(type, OfPoint3D) || type == OfIPoint3D) { from.SendGump(new SetPoint3DGump(prop, from, m_Object, this)); } - else if (IsType(type, OfPoint2D)) + else if (IsType(type, OfPoint2D) || type == OfIPoint2D) { from.SendGump(new SetPoint2DGump(prop, from, m_Object, this)); } diff --git a/Projects/UOContent/Gumps/Props/SetPoint2DGump.cs b/Projects/UOContent/Gumps/Props/SetPoint2DGump.cs index 35fd70103..5007bef22 100644 --- a/Projects/UOContent/Gumps/Props/SetPoint2DGump.cs +++ b/Projects/UOContent/Gumps/Props/SetPoint2DGump.cs @@ -26,7 +26,7 @@ namespace Server.Gumps m_Mobile = mobile; m_Object = o; - var p = (Point2D)(prop?.GetValue(o, null) ?? new Point2D()); + var p = prop?.GetValue(o, null) is IPoint2D current ? new Point2D(current) : new Point2D(); AddPage(0); diff --git a/Projects/UOContent/Gumps/Props/SetPoint3DGump.cs b/Projects/UOContent/Gumps/Props/SetPoint3DGump.cs index b0707bc3d..911f01a86 100644 --- a/Projects/UOContent/Gumps/Props/SetPoint3DGump.cs +++ b/Projects/UOContent/Gumps/Props/SetPoint3DGump.cs @@ -27,7 +27,7 @@ namespace Server.Gumps m_Mobile = mobile; m_Object = o; - var p = (Point3D)(prop?.GetValue(o, null) ?? new Point3D()); + var p = prop?.GetValue(o, null) is IPoint3D current ? new Point3D(current) : new Point3D(); AddPage(0); diff --git a/Projects/UOContent/Utilities/Types.cs b/Projects/UOContent/Utilities/Types.cs index 184773e84..b77511e96 100644 --- a/Projects/UOContent/Utilities/Types.cs +++ b/Projects/UOContent/Utilities/Types.cs @@ -33,6 +33,8 @@ namespace Server public static readonly Type OfTimeSpan = typeof(TimeSpan); public static readonly Type OfPoint3D = typeof(Point3D); public static readonly Type OfPoint2D = typeof(Point2D); + public static readonly Type OfIPoint3D = typeof(IPoint3D); + public static readonly Type OfIPoint2D = typeof(IPoint2D); public static readonly Type OfEnum = typeof(Enum); public static readonly Type OfType = typeof(Type); @@ -278,6 +280,23 @@ namespace Server return null; } + if (type == OfIPoint3D || type == OfIPoint2D) + { + if (Point3D.TryParse(value, null, out var p3)) + { + constructed = p3; + return null; + } + + if (type == OfIPoint2D && Point2D.TryParse(value, null, out var p2)) + { + constructed = p2; + return null; + } + + return "That is not properly formatted."; + } + if (IsType(type, OfBool)) { if (bool.TryParse(value, out var parsed))