erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

fix(chat): CH4 review fixes — allegiance ownership guard, house-abandon confirmation

Browse source 
Blocker 1: an unrecognized "@allegiance <sub>" subcommand escaped
TryMatchAllegiance (which only claimed "info"/"hometown") and fell through
the unregistered-tag channel fallback, broadcasting the raw subcommand
text to the Allegiance chat channel (0x02000000). Retail's own
DoAllegiance never reaches DoChannelCommand for an unrecognized
subcommand — it claims the whole verb and prints its own client-local
refusal. TryMatchAllegiance now claims "allegiance"/"all" unconditionally
and shows retail's "Please see @help Allegiance..." text; ChatCommandRouter
also gained a blanket RetailClientCommandCatalog.KnownVerbs ownership
guard in TryDispatchChannelFallback as defense in depth.

Blocker 2: "@house abandon" sent 0x021F immediately with no confirmation.
Retail runs a real two-stage dialog before Event_AbandonHouse(); ported
both verbatim strings and chained two ShowConfirmation calls.

Should-fixes: a bare unregistered tag with no text now passes through
silently instead of showing a refusal that belongs to a different retail
function; @join/@leave update RuntimeCharacterOptionsState locally (new
SetOptionBit) before the wire push so the Turbine membership gate stops
refusing a just-joined room; @permit accepts multi-word names; @clist/
@on/@off validate shape only and raise WeenieError 0x422 for an unknown
tag; @mr/@pr help text is now the verbatim retail strings; corrected
issue #360, register row TS-68, the campaign doc's B.7 note, and a stale
RetailChannelTagTable comment; filed issue #363 + register row AP-183 for
the deferred error-typing debt.

Nits: fixed TryMatchHouse's stale doc comment, the AP-182/@title "stores
the value" comments (the binding is a no-op), IsUnregisteredFallbackTag's
olthoi false-positive, added /g and /rp binding-level conformance pins,
made @index ignore extra arguments, and noted the six removed invented
verbs in ISSUES.md.

Suite: 12,216 passed / 4 skipped / 0 failed (Release), up from CH4's
12,190/4/0 — net +26 tests, no removals.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-09 21:59:35 +02:00
parent 090825e703
commit 724ef2d389
17 changed files with 853 additions and 85 deletions
Download patch file Download diff file Expand all files Collapse all files

122
src/AcDream.UI.Abstractions/Panels/Chat/RetailClientCommandCatalog.cs
View file

@ -238,8 +238,11 @@ public static class RetailClientCommandCatalog
// ClientCommunicationSystem::DoTitle @ 0x0057A640. Exact retail help:
// acclient_2013_pseudo_c.txt:1031162 (data_7df2c4) — "@title <new
// title> - Sets the title of the popup chat window.\n". No confirmation
// text was found at the success site; acdream stores the title but has
// no title-bar chrome to render it yet (AP-182).
// text was found at the success site; acdream's binding is a pure
// no-op (the value is neither stored nor consumed — no title-bar
// chrome exists to render it yet, AP-182; corrected 2026-08-09 at the
// CH4 REJECT-review nit 11, which found the earlier "acdream stores
// the title" wording false).
private static readonly Definition SetTitle = AnyArguments(
ClientCommandId.SetChatTitle,
"/title <new title>",
@ -305,7 +308,14 @@ public static class RetailClientCommandCatalog
ValidateArguments: static arguments => JoinLeaveTags.ContainsKey(arguments.Trim()));
// ClientCommunicationSystem::DoPermit @ 0x005785A0. Exact retail help:
// acclient_2013_pseudo_c.txt:1030850-1030852 (data_7dbac8).
// acclient_2013_pseudo_c.txt:1030850-1030852 (data_7dbac8). CH4
// REJECT-review SHOULD-FIX 5 (2026-08-09): DoPermit joins every token
// after "add"/"remove" into the name (JoinArgsAsName) so a multi-word
// character name works — "@permit add Aunt Agatha" grants Aunt Agatha,
// not just "Aunt". The old exactly-2-tokens gate rejected that input
// outright; the shape check now only requires a mode word plus AT
// LEAST one more token, and ExecutePermit
// (ClientCommandController.cs) joins the remainder.
private static readonly Definition Permit = new(
ClientCommandId.Permit,
Usage: "/permit <add|remove> <name>",
@ -313,7 +323,7 @@ public static class RetailClientCommandCatalog
ValidateArguments: static arguments =>
{
string[] parts = arguments.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries);
return parts.Length == 2
return parts.Length >= 2
&& (parts[0].Equals("add", StringComparison.OrdinalIgnoreCase)
|| parts[0].Equals("remove", StringComparison.OrdinalIgnoreCase));
});
@ -344,36 +354,51 @@ public static class RetailClientCommandCatalog
// ClientCommunicationSystem::DoChannelIndex @ 0x0056E640. No help
// string was extracted for the bare form; the verb is admin/advocate/
// PSR gated server-side (GameActionChannelIndex.Handle).
private static readonly Definition IndexChannels = NoArguments(
// PSR gated server-side (GameActionChannelIndex.Handle). CH4
// REJECT-review nit 14 (2026-08-09): DoChannelIndex ignores its argc —
// "@index foo" sends the SAME Event_ChannelIndex() as bare "@index" —
// so acdream must accept (and discard) any arguments too, not just none.
private static readonly Definition IndexChannels = AnyArguments(
ClientCommandId.IndexChannels,
"/index",
"@index - Requests the channel index (restricted).");
// ClientCommunicationSystem::DoChannelList @ 0x0057A9B0. Exact retail
// no-arg text: acclient_2013_pseudo_c.txt:1031202 (data_7dfaf8)
// "Please specify the channel name."
// "Please specify the channel name." CH4 REJECT-review SHOULD-FIX 6
// (2026-08-09): retail's own argc check is "!= 1" — a resolved-but-
// UNKNOWN tag still reaches the handler and raises
// HandleFailureEvent(0x422) ("That channel doesn't exist.",
// WeenieErrorMessages[0x422]); only a MISSING or MULTI-WORD argument
// prints this usage line locally. Using tag resolution itself as the
// argument-shape gate (the old behavior) silently swallowed an unknown
// tag instead of raising 0x422 — see ClientCommandController's
// dispatch (ListChannel/OnChannel/OffChannel cases) for the
// ShowWeenieError(0x422) call this shape-only gate now allows through.
private static readonly Definition ListChannel = new(
ClientCommandId.ListChannel,
Usage: "/clist <channel>",
HelpText: "@clist <channel> - Requests the member list of a channel (restricted).",
ValidateArguments: static arguments => RetailChannelTagTable.TryResolve(arguments.Trim(), out _),
ValidateArguments: static arguments => IsSingleToken(arguments),
InvalidArgumentsText: "Please specify the channel name.");
private static readonly Definition OnChannel = new(
ClientCommandId.OnChannel,
Usage: "/on <channel>",
HelpText: "@on <channel> - Joins a channel (restricted).",
ValidateArguments: static arguments => RetailChannelTagTable.TryResolve(arguments.Trim(), out _),
ValidateArguments: static arguments => IsSingleToken(arguments),
InvalidArgumentsText: "Please specify the channel name.");
private static readonly Definition OffChannel = new(
ClientCommandId.OffChannel,
Usage: "/off <channel>",
HelpText: "@off <channel> - Leaves a channel (restricted).",
ValidateArguments: static arguments => RetailChannelTagTable.TryResolve(arguments.Trim(), out _),
ValidateArguments: static arguments => IsSingleToken(arguments),
InvalidArgumentsText: "Please specify the channel name.");
private static bool IsSingleToken(string arguments) =>
arguments.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries).Length == 1;
// GameActionRecallAllegianceHometown.Handle. Exact retail help:
// acclient_2013_pseudo_c.txt:1031230 — "@allegiance hometown -
// Recalls you to your allegiance bindstone, if your allegiance has
@ -392,6 +417,30 @@ public static class RetailClientCommandCatalog
"/allegiance info [name]",
"@allegiance info <name> - Requests information on a member of your allegiance.");
// ClientCommunicationSystem::DoAllegiance @ 0x0057D5A0. Exact retail
// text: acclient_2013_pseudo_c.txt:1031375 (data_7e0bd0) — "Please see
// @help Allegiance for more information on how to use this command.".
// Printed at label_57da4b (0x0057DA4B) whenever NO subcommand string
// matches ANY of the 12 retail dispatches (boot/info/chat/broadcast/
// ban/officer/title/hometown/ho/motd/name/lock/house) — retail keeps
// this ENTIRELY client-side; DoAllegiance never falls through to
// DoChannelCommand or the server for an unrecognized subcommand. CH4
// REJECT-review Blocker 1 (2026-08-09): acdream previously let an
// unmatched subcommand escape TryMatchAllegiance (return false, "not
// owned"), which fell all the way through to the unregistered-tag
// channel-fallback and broadcast the raw subcommand text ("boot Bob")
// to the legacy Allegiance channel (0x02000000) — a real chat-visible
// bug. TryMatchAllegiance below now claims ownership of "allegiance"/
// "all" UNCONDITIONALLY, exactly like retail's registered-command hash
// table does, and shows this refusal for every subcommand beyond the
// 2 ported ones (info/hometown/ho — TS-68 tracks the other 10).
private static readonly Definition AllegianceUnrecognizedSubcommand = new(
ClientCommandId.AllegianceUnrecognizedSubcommand,
Usage: "/allegiance <sub>",
HelpText: "Please see @help Allegiance for more information on how to use this command.",
ValidateArguments: static _ => false,
InvalidArgumentsText: "Please see @help Allegiance for more information on how to use this command.");
private static readonly FrozenDictionary<string, Definition> ByVerb =
new Dictionary<string, Definition>(StringComparer.OrdinalIgnoreCase)
{
@ -520,14 +569,17 @@ public static class RetailClientCommandCatalog
/// Retail's real <c>DoHouse @ 0x00580860</c> handles 15 subcommands
/// (see the registry doc §2.5b) locally; acdream Campaign CH slice CH4
/// (2026-08-09) ports 4 of them (recall/re, mansion_recall/alleg_recall/
/// ma, abandon) plus the pre-existing HasValidArguments==false swallow
/// for a MISSPELLED recall variant. Every OTHER subcommand — open,
/// close, storage, remove, boot, boot_all, remove_all, guest, available,
/// hooks, on, off — is NOT yet ported (TS-68) and must reach ACE
/// (which replies "Unknown command") rather than being swallowed
/// locally with a wrong usage message — the Tier-1 #4 fix from the
/// command-registry doc. Returning <c>false</c> here lets
/// <see cref="ChatCommandRouter"/> fall through to server passthrough.
/// ma, abandon). Every OTHER subcommand — open, close, storage, remove,
/// boot, boot_all, remove_all, guest, available, hooks, on, off, and
/// any misspelling of the 4 ported ones — returns <c>false</c>
/// uniformly (there is no separate local-swallow branch; CH4
/// REJECT-review nit 10, 2026-08-09, corrected this comment, which
/// previously described a swallow path that does not exist in the code
/// below), letting <see cref="ChatCommandRouter"/> fall through to
/// server passthrough (ACE replies "Unknown command") rather than
/// being swallowed locally with a wrong usage message — the Tier-1 #4
/// fix from the command-registry doc. The 12 unported subcommands are
/// tracked by TS-68.
/// </summary>
private static bool TryMatchHouse(string arguments, out Match match)
{
@ -556,14 +608,26 @@ public static class RetailClientCommandCatalog
/// <c>@allegiance &lt;sub&gt;</c> / <c>@all &lt;sub&gt;</c> dispatcher.
/// Retail's real <c>DoAllegiance @ 0x0057D5A0</c> handles 12
/// subcommands (see the registry doc §2.5) locally; acdream Campaign CH
/// slice CH4 (2026-08-09) ports 2 of them (info, hometown/ho). Every
/// OTHER subcommand — boot, ban, officer, title, name, lock, house,
/// motd, chat, broadcast — is NOT yet ported (TS-68) and falls through
/// to server passthrough, same reasoning as <see cref="TryMatchHouse"/>.
/// slice CH4 (2026-08-09) ports 2 of them (info, hometown/ho).
/// </summary>
/// <remarks>
/// <b>CH4 REJECT-review Blocker 1 correction (2026-08-09):</b> every
/// OTHER subcommand — boot, ban, officer, title, name, lock, house,
/// motd, chat, broadcast, or garbage — is NOT yet ported (TS-68), but
/// unlike <see cref="TryMatchHouse"/> this method NEVER returns
/// <c>false</c> for the "allegiance"/"all" verb: retail's own
/// <c>DoAllegiance</c> claims the ENTIRE verb unconditionally and
/// prints its own client-local refusal
/// (<see cref="AllegianceUnrecognizedSubcommand"/>) for an unrecognized
/// subcommand — it never falls through to <c>DoChannelCommand</c> or
/// the server. The original CH4 implementation returned <c>false</c>
/// here (matching <see cref="TryMatchHouse"/>'s reasoning), which let
/// an unmatched subcommand escape all the way to the unregistered-tag
/// channel-fallback and broadcast the raw text to the Allegiance
/// channel — a real bug, not merely an incomplete port.
/// </remarks>
private static bool TryMatchAllegiance(string arguments, out Match match)
{
match = default;
int separator = IndexOfWhitespace(arguments);
string subcommand = separator < 0 ? arguments : arguments[..separator];
string rest = separator < 0 ? string.Empty : arguments[(separator + 1)..].Trim();
@ -591,7 +655,17 @@ public static class RetailClientCommandCatalog
return true;
}
return false;
// Every other subcommand (or none at all) — claim ownership
// anyway and show retail's own refusal text. See the remarks
// above; this is what stops "allegiance"/"all" from ever reaching
// ChatCommandRouter's channel-fallback or server-passthrough path.
match = new Match(
AllegianceUnrecognizedSubcommand.Command,
arguments,
AllegianceUnrecognizedSubcommand.Usage,
HasValidArguments: false,
AllegianceUnrecognizedSubcommand.InvalidArgumentsText);
return true;
}
/// <summary>Help line generated from the same definition routing uses.</summary>