ModernUO/Projects/Server/Text/TextDefinition.cs
Kamron Batman c9831b9680
fix: TextDefinition was uneditable in the props gump, and where parsed constants its own way (#2624)
## 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.
2026-09-10 19:36:30 -07:00

196 lines
6.2 KiB
C#

/*************************************************************************
* ModernUO *
* Copyright 2019-2026 - ModernUO Development Team *
* Email: hi@modernuo.com *
* File: TextDefinition.cs *
* *
* This program is free software: you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation, either version 3 of the License, or *
* (at your option) any later version. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System;
using System.Runtime.CompilerServices;
namespace Server;
[PropertyObject]
public class TextDefinition : IEquatable<object>, IEquatable<TextDefinition>, ISpanParsable<TextDefinition>
{
public static readonly TextDefinition Empty = new();
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TextDefinition Of(int number) => Of(number, null);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TextDefinition Of(string text) => Of(0, text);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TextDefinition Of(ReadOnlySpan<char> text) => Of(0, text);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TextDefinition Of(int number, string text) => new(number, text);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TextDefinition Of(int number, ReadOnlySpan<char> text) => new(number, text);
private TextDefinition()
{
}
private TextDefinition(int number, string text)
{
Number = number;
String = text;
}
private TextDefinition(int number, ReadOnlySpan<char> text)
{
Number = number;
String = text.ToString();
}
[CommandProperty(AccessLevel.GameMaster)]
public int Number { get; }
[CommandProperty(AccessLevel.GameMaster)]
public string String { get; }
public bool IsEmpty => Number <= 0 && String == null;
public override string ToString() => Number > 0 ? $"#{Number}" : String ?? "";
public string Format() =>
Number > 0 ? $"{Number} (0x{Number:X})" :
String != null ? $"\"{String}\"" : null;
/// <summary>
/// The editable text form. Quotes a string that <c>TryParse</c> would not read back unchanged;
/// the check is a real round trip so it cannot drift from the parser.
/// </summary>
public string GetValue()
{
if (Number > 0)
{
return Number.ToString();
}
if (String == null)
{
return "";
}
return TryParse(String, null, out var parsed) && parsed.Number == 0 && parsed.String == String
? String
: $"@\"{String}\"";
}
public static implicit operator TextDefinition(int v) => Of(v);
public static implicit operator TextDefinition(string s) => Of(s);
public static implicit operator int(TextDefinition m) => m?.Number ?? 0;
public static implicit operator string(TextDefinition m) => m?.String;
public void Deconstruct(out int number, out string s)
{
if (Number > 0)
{
number = Number;
s = null;
}
else
{
number = 0;
s = String;
}
}
public override bool Equals(object obj) => Equals(obj as TextDefinition);
public bool Equals(TextDefinition other)
{
if (ReferenceEquals(null, other))
{
return false;
}
if (ReferenceEquals(this, other))
{
return true;
}
if (Number > 0 || other.Number > 0)
{
return Number == other.Number;
}
return String == other.String;
}
public override int GetHashCode() => Number > 0 ? HashCode.Combine(Number) : HashCode.Combine(String);
public static bool operator ==(TextDefinition left, TextDefinition right) => Equals(left, right);
public static bool operator !=(TextDefinition left, TextDefinition right) => !Equals(left, right);
public static TextDefinition Parse(string value) => value == null ? null : Parse(value.AsSpan(), null);
public static TextDefinition Parse(string s, IFormatProvider provider) => Parse(s.AsSpan(), provider);
public static bool TryParse(string s, IFormatProvider provider, out TextDefinition result) =>
TryParse(s.AsSpan(), provider, out result);
public static TextDefinition Parse(ReadOnlySpan<char> s, IFormatProvider provider)
{
TryParse(s, provider, out var result);
return result;
}
/// <summary>
/// <c>#1234</c> or a bare <c>1234</c>/<c>0x4D2</c> is a cliloc; <c>@"1234"</c> is the literal
/// text. Always succeeds -- anything that is not a cliloc is a string.
/// See <c>dev-docs/generic-commands.md</c>.
/// </summary>
public static bool TryParse(ReadOnlySpan<char> s, IFormatProvider provider, out TextDefinition result)
{
if (TryGetQuotedLiteral(s, out var literal))
{
result = Of(literal);
return true;
}
if (s.Length > 1 && s[0] == '#' && Utility.ToInt32(s[1..], out var marked))
{
result = Of(marked);
return true;
}
if (Utility.ToInt32(s, out var label))
{
result = Of(label);
return true;
}
// We don't trim
result = Of(s);
return true;
}
private static bool TryGetQuotedLiteral(ReadOnlySpan<char> s, out ReadOnlySpan<char> literal)
{
if (s.Length >= 3 && s[0] == '@' && s[1] == '"' && s[^1] == '"')
{
literal = s[2..^1];
return true;
}
literal = default;
return false;
}
}