## 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.
## Pressing `>` on a TextDefinition did nothing useful
`#1217` moved `TextDefinition` into the Server project for serialization support and added `[PropertyObject]` along the way. `PropsGump` checks that attribute **before** its parsable fallback, so from that commit on the button either drilled into a read-only `Number`/`String` page (both get-only since `#1221`, so no `>` buttons — a dead end) or, when the value was null, re-sent the same page and looked inert.
Worth being clear that `#765` — which replaced the hand-maintained type list with a generic `IsParsable` branch — was **not** the regression. At that commit `TextDefinition` was `[Parsable]` only, fell through to the new branch, and the editor worked. Only the later attribute hijacked the routing.
`TextDefinition` is now caught ahead of the `[PropertyObject]` branch and routed to `SetGump`, which has had `TextDefinition`-specific code at line 35 all along.
## `0` and `"0"` could not be told apart
`Commands.Split` strips quotes before any parser runs, so this was never fixable in `TextDefinition.Parse` alone — the information is gone two layers earlier. The markers now travel inside the value:
| Input | Result |
|---|---|
| `1060847` | cliloc (unchanged) |
| `#1060847` | cliloc, explicit — the form `ToString()` already writes |
| `@"1060847"` | the literal string |
| `hello` | string (unchanged) |
| `(-null-)` | null (unchanged) |
`GetValue()` quotes a string that would not survive the trip back. That check is a real round trip through the parser rather than a pattern match, so it cannot drift from it. `Types.TryParse` decodes the same escape for plain strings — which is what `[get` has always written for the literal `"null"` without `[set` ever reading it back.
**Saved data is untouched.** Serialization reads a discriminated flag plus int/string (`IGenericReader.cs:175`) and the JSON converter switches on token type; neither calls `Parse`, so a stored string `"1060847"` stays a string and spawner JSON is unchanged. The new syntax stays in the text and command layer, where it reaches `[set`, `[add`, spawner props, the props gump and Advanced Search.
## `where` still parsed its constants its own way
`#2625` carried the hand-rolled constant parser across to `PropertyExpressions.Parse` unchanged. It looks for a static `Parse` overload on the property type and gives up if there is none — and `Type` and `IEntity` have neither:
```
where Subject Kind = Static -> Unable to convert string "Static" into type 'System.Type'.
where Subject Owner = 0x1 -> Unable to convert string "0x1" into type 'Server.Mobile'.
```
Both resolve fine for every other command. Routing through `Types.TryParse` picks up type-name lookup, entity resolution by serial, and the `@"..."` literal convention, so a value that works in one place now works everywhere. It also deletes the duplicate parser (−56 lines in `PropertyExpressions`).
Two things are load-bearing and preserved. `where` has always spelled a null constant as a bare `null`, where `[set` uses `(-null-)`; the shared parser reads a bare `null` as the text, so the null case is handled ahead of it and `= null` keeps meaning null. And nullable targets still unwrap first, so `where <int? prop> = 5` is unaffected.
Bare and hex integers, enums, bools, strings, `Map`, and TextDefinition's `#` / `@"..."` markers all resolve exactly as before — 13 of the 15 tests in `WhereConstantParsingTests` pin that and passed before the change as well as after.
## Documentation
The generic command system had no documentation anywhere in the repo: scopes, `where` and its operators, dot notation, `order by` / `distinct` / `limit`, the value and quoting syntax, `[interface` and `[batch` were all learn-by-reading-the-source. `dev-docs/generic-commands.md` covers them, linked from `CLAUDE.md` and `commands-targeting.md`, and points at <https://muo.gg/commands> for the per-command list rather than duplicating it.
Two behaviours it records that were news to me while writing it:
- `Contained` honours conditions on the normal command path but never sets `SupportsConditionals`, and `[batch` is the only place that reads the flag — so the same condition works typed directly and is refused under batch.
- `Multi`, `Single`, `Self` and `Serial` do not parse modifiers at all, so a `where` clause there is passed to the command as ordinary arguments rather than rejected.
Comments in the changed code were trimmed to what is not evident from the code itself; the rationale they carried is either in the doc now or in the commit that introduced it.
## Behavior changes worth a reviewer's attention
Two things go beyond strict bug-fixing, both deliberate:
- **Hex now parses into a TextDefinition.** Consolidating the three `Parse` overloads onto one span codec means `[set Message 0x102CE7` is cliloc 1060847 rather than the string. Previously only the 1-arg `Parse(string)` did hex and nothing called it. This makes the props gump's own `1060847 (0x102CE7)` display typeable.
- **`@"..."` decodes generally for strings**, not only the exact token `@"null"`. So `[set Name @"hello"` sets `hello`. A half-working escape seemed more surprising than a general one, and the `where` path already decoded it — but narrowing it back is a one-line change if preferred.
## Testing
40 tests, each written first and watched fail for the right reason. They cover the props gump routing (populated and null), cliloc/string/quoted parsing and `GetValue` round trips, `where` constant resolution for Type- and entity-valued properties, and the bare-`null` vs `@"null"` distinction that must not drift.
One sort case is added that #2625 left uncovered: ordering a value-typed chain whose intermediate is null, which reads as `default(int)` rather than throwing. Two other tests from the pre-rebase branch were dropped as duplicates of `ConditionalCompilerEdgeTests`.
Rebased onto `535a09899`. `dotnet build` clean, 0 warnings. **902 UOContent** and **869 Server** tests pass, 0 failures.
## Known gaps
- `Nullable<T>` comparisons work via #2625's lifted operators; nothing here changes that.
- Spawner **Params** are split on plain spaces (`BaseSpawner.cs:1035`) rather than through `Commands.Split`, so a constructor argument still cannot contain one. Untouched here — a tokenizer issue, not a TextDefinition one.
- `Types.ParseStringNumericParamTypes` is now unreferenced. Left in place because it is public and shards may use it.
### Summary
- Fixes `[AdvancedSearch` being accessible by players 😱
- Adds `[GenCommands` to generate the same commands html page on https://muo.gg/commands.
- Fixes `[helpinfo` so all commands properly show up!
> [!WARNING]
> ### Developer Warning:
> Commands must now be registered in the `Configure` bootup phase.
> If a command is not registered early enough, it may not be available to systems like [helpinfo
> that cache their information.
> [!NOTE]
> ### Developer Note:
> Various commands related to generating content have been changed to _Developer_ and above access level.
### Screenshots
<img width="673" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/b105b5c9-5eb4-4ace-93ff-1bfb31e7132f">
<img width="547" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/e97487e8-47a5-4aa7-89cc-9fe3deda584d">
### Summary
- Removes old death packet that isn't used. Doubtful this causes issues with clients that are v4+.
- Removes duplicate incoming packets. Again, probably to fix some old client issues, doubtful it affects clients v4+.
- Fixes setting serials and entities in props/commands. Note: Disabled setting `Parent` since the new sector code has issues. We shouldn't rely on it anyway!
- Reverts a recent change to healthbars that should not have been made. Oops!
- [X] Removes static freezing/unfreezing. Use other tools for this.
- [X] Cleans up IPoint3D allocations
- [X] Removes IPoint3D constructors since the compiler may not optimize the constructor path and allow allocations.
Note: Instead of `new Point3D(m)`, do something like `new Point3D(m.Location)`. Sorry for the inconvenience. In the long run this will prevent abuse of hot paths that will cause performance issues.
* Removes BodyValue
* Removes BodyPicker gump
* Fixes setting Body and BodyMod in props gump
* Props will now look for an `implicit cast` in order to find a way to allow you to modify values with the props gump.
* Fixed `[self set body 0x190` so it works too.
- [X] Adds a property to `CommandProperty` to allow modifying `PropertyObject` where there is no property setter that is accessible.
- [X] Updates AosAttributes and friends
Example of how to make this work:
```cs
[CommandProperty(AccessLevel.GameMaster, canModify: true)]
public AosWeaponAttributes WeaponAttributes { get; private set; }
```
This fix is necessary because in release mode the optimizer will sometimes _entirely remove the setter_ because it knows it can do something that is more efficient _when the property setter is private_. RunUO got around this by declaring a public setter explicitly that _was empty_.
### Screenshot
<img width="313" alt="Screen Shot 2021-05-15 at 11 34 01 PM" src="https://user-images.githubusercontent.com/3953314/118387955-49293c00-b5d6-11eb-8f31-a3b5e31bd18b.png">
- [X] Adds `Respawn` command
- [X] Adds `EditSpawner` command
- Usage: `[global editspawner <type> <arguments> set <properties>`
- This command replaces the arguments (optional) and properties of the affected spawners for that type
- [X] Adds `SpawnProps` command
- Usage: `[area spawnprops`
- This command brings up a gump to modify targeted spawners based on the targeted entity.
- To do this, the props/values are copied and then used to construct a properties list for the spawner entry.
Closes#531
SpanWriter now has an argument that allows it to be resizable.
```cs
public SpanWriter(Span<byte> initialBuffer, bool resize = false)
public SpanWriter(int initialCapacity, bool resize = false)
```
If a SpanWriter is set to be resizable, or given an initial capacity instead of a buffer, it must be disposed:
```cs
public static void SomeMethod()
{
using var writer = new SpanWriter(stackalloc byte[512], true);
// stuff
// Automatic Dispose of writer
}
```
This is because the SpanWriter uses `ArrayPool<byte>.Shared` _rented buffers_ to resize. If the writer is not disposed, those buffers will never be reused resulting in a _memory leak_.
It is possible that the SpanWriter will outright ditch the initial buffer if resize is set to true and growing is needed. To check/account for that we can do the following:
```cs
public static void SomeMethod()
{
Span<byte> span = stackalloc byte[512];
using var writer = new SpanWriter(span, true);
// write some stuff that causes the span to grow
span = writer.RawBuffer;
// Do stuff with the span
}
```
Make sure you don't accidentally `Dispose()` the writer or use the `RawBuffer` outside of the `using` block. If you do, bad things will happen! (NullPointerException, or a fresh SpanWriter with no buffer, depending on the situation).
SpanWriter can also now be used with a fixed statement since it has a `PinnableReference()` function.
- [X] Removes some string allocations (e.g. split)
- [X] Optimizes some collections
- [X] Converts insensitive to extension methods of built-ins.
- [X] Adds ordinal (case sensitive) string helpers
- [X] Fixes conditionals for in-game commands so they use Ordinal comparisons.
- [X] Replaces ToLower.Contains with InsensitiveContains
- [X] Adds ValueStringBuilder
- [X] Implements ValueStringBuilder in a few places where it makes sense
- [X] Removes the redundant Wrap function and replaces it with an optimized version
- [X] Fixes list conversions in Utility
Closes#351
Bumps release version
- [X] Fixes several bugs
- [X] Updates more ordinal issues
- [X] Cleans up the code a bit
- [X] Turns classes static that should have been
- [X] Changes TcpServer.Instances to a HashSet
Bumps release version
