docs: rewrite README for the TextCommandCallingArgs API

FireBefore continued invoking every registered listener even after one had set Cancel, running callbacks against a command that was already dead. Break out of the loop as soon as Cancel is set.

CommandHook/ChatCommandApiPatch.cs

@@ -0,0 +1,91 @@
+using System;
+using HarmonyLib;
+using Vintagestory.API.Common;
+using Vintagestory.Common;
+
+namespace CommandHook;
+
+/// <summary>
+/// Harmony prefix patch on <c>ChatCommandApi.Execute</c>. This is the only place
+/// CommandHook hooks into the game, everything else in the mod feeds into this.
+/// </summary>
+[HarmonyPatch(typeof(ChatCommandApi))]
+public static class ChatCommandApiPatch
+{
+    /// <summary>
+    /// Runs before the game's own command execution. Fires Before listeners for
+    /// <paramref name="commandName"/> and, if none of them cancel, lets the
+    /// original <c>Execute</c> run and wraps <paramref name="onCommandComplete"/>
+    /// so After listeners fire once a result exists.
+    /// </summary>
+    /// <remarks>
+    /// Runs for both invocation paths, server console and a connected player
+    /// typing in chat, since both go through <c>Execute</c> the same way
+    /// internally.
+    /// <para>
+    /// If a Before listener returns a non-null <see cref="TextCommandResult"/>,
+    /// this returns false to stop the original <c>Execute</c> from running at
+    /// all, and invokes <paramref name="onCommandComplete"/> directly with that
+    /// result so the caller sees whatever the listener intended (a real error
+    /// message, a silent <see cref="TextCommandResult.Deferred"/>, etc). After
+    /// listeners are not fired in this case.
+    /// </para>
+    /// <para>
+    /// If nothing cancels, this returns true and lets the original <c>Execute</c>
+    /// run, but first replaces <paramref name="onCommandComplete"/> with a wrapper
+    /// that calls the original callback, then fires After. The original callback
+    /// always runs first, so nothing about the vanilla command flow changes from
+    /// the caller's point of view.
+    /// </para>
+    /// </remarks>
+    /// <param name="commandName">The command name being executed, without the leading slash.</param>
+    /// <param name="args">
+    /// The calling args supplied by the game. Passed straight through to listeners,
+    /// live and unmodified, the engine's own object for this invocation.
+    /// </param>
+    /// <param name="onCommandComplete">
+    /// The completion callback supplied by the game. Replaced with a wrapper that
+    /// fires After listeners after invoking the original.
+    /// </param>
+    /// <returns>
+    /// False to skip the original <c>Execute</c> (a Before listener cancelled it),
+    /// true to let it run normally.
+    /// </returns>
+    [HarmonyPatch(
+        "Execute",
+        new[] { typeof(string), typeof(TextCommandCallingArgs), typeof(Action<TextCommandResult>) }
+    )]
+    [HarmonyPrefix]
+    public static bool Prefix(
+        string commandName,
+        TextCommandCallingArgs args,
+        ref Action<TextCommandResult> onCommandComplete
+    )
+    {
+        var system = CommandHookModSystem.Instance;
+        // Instance is null before StartServerSide runs or after Dispose. Harmony
+        // patches stay applied for the process lifetime, so this null check is
+        // what actually guards against firing into a torn-down mod.
+        if (system == null)
+            return true;
+
+        var cancelResult = system.FireBefore(commandName, args);
+        if (cancelResult != null)
+        {
+            onCommandComplete?.Invoke(cancelResult);
+            return false;
+        }
+
+        // Not cancelled. Capture the game's own callback before replacing it,
+        // since we still need to call it, this patch only adds behavior around
+        // the vanilla flow, it never removes the vanilla completion handling.
+        var original = onCommandComplete;
+        onCommandComplete = result =>
+        {
+            original?.Invoke(result);
+            system.FireAfter(commandName, args, result);
+        };
+
+        return true;
+    }
+}

CommandHook/CommandData.cs

@@ -1,29 +0,0 @@
-using Vintagestory.API.Server;
-
-namespace CommandHook;
-
-public struct CommandData
-{
-    public readonly IServerPlayer? Sender;
-    public readonly string FullCommand;
-    public readonly string CommandName;
-    private byte _flags;
-
-    private const byte FlagIsPlayerCommand = 1 << 0;
-    private const byte FlagCancel = 1 << 1;
-
-    public bool IsPlayerCommand => (_flags & FlagIsPlayerCommand) != 0;
-    public bool Cancel
-    {
-        get => (_flags & FlagCancel) != 0;
-        set => _flags = value ? (byte)(_flags | FlagCancel) : (byte)(_flags & ~FlagCancel);
-    }
-
-    public CommandData(IServerPlayer? sender, string fullCommand)
-    {
-        Sender = sender;
-        FullCommand = fullCommand.Length > 0 ? fullCommand.Trim() : string.Empty;
-        CommandName = FullCommand.Length > 1 ? FullCommand[1..].Split(' ')[0] : string.Empty;
-        _flags = sender != null ? FlagIsPlayerCommand : (byte)0;
-    }
-}

CommandHook/CommandDelegate.cs

@@ -0,0 +1,30 @@
+using Vintagestory.API.Common;
+
+namespace CommandHook;
+
+/// <summary>
+/// Invoked before a watched command runs. Return a non-null
+/// <see cref="TextCommandResult"/> to cancel the command and report that
+/// result to the caller (e.g. <see cref="TextCommandResult.Error(string, string)"/>
+/// for a visible reason, or <see cref="TextCommandResult.Deferred"/> to cancel
+/// silently). Return null to let the command run normally.
+/// </summary>
+/// <param name="callingArgs">
+/// The live calling args for this invocation, supplied directly by the engine.
+/// </param>
+public delegate TextCommandResult? CommandBeforeDelegate(TextCommandCallingArgs callingArgs);
+
+/// <summary>
+/// Invoked after a watched command has run. Not invoked if a Before listener
+/// (for this command, any mod) cancelled it.
+/// </summary>
+/// <param name="callingArgs">
+/// The same live calling args object passed to Before. By now the real
+/// command handler has had full access to it, and what's safe to read
+/// depends on how that command is implemented internally.
+/// </param>
+/// <param name="result">The result the command produced.</param>
+public delegate void CommandAfterDelegate(
+    TextCommandCallingArgs callingArgs,
+    TextCommandResult result
+);

CommandHook/CommandHookDelegate.cs

@@ -1,3 +0,0 @@
-namespace CommandHook;
-
-public delegate void CommandHookDelegate(ref CommandData data);

CommandHook/CommandHookModSystem.cs

@@ -1,27 +1,220 @@
+using System;
+using System.Collections.Frozen;
 using System.Collections.Generic;
+using HarmonyLib;
 using Vintagestory.API.Common;
 using Vintagestory.API.Server;
 
 namespace CommandHook;
 
+/// <summary>
+/// Server-side event bus for chat commands. Other mods register an
+/// <see cref="ICommandHookListener"/> here to get Before/After callbacks on
+/// specific command names, namespaced by <see cref="ICommandHookListener.ModId"/>
+/// so multiple mods can watch the same command without stepping on each other.
+/// </summary>
+/// <remarks>
+/// Patches <c>ChatCommandApi.Execute</c> via Harmony. Server only, see
+/// <see cref="ShouldLoad"/>. A listener's <see cref="ICommandHookListener.Commands"/>
+/// and <see cref="ICommandHookListener.Registration"/> are read at registration
+/// time and cached in a <see cref="System.Collections.Frozen.FrozenDictionary{TKey,TValue}"/>
+/// for fast lookup on the command dispatch path. If you change which commands a
+/// listener watches, call <see cref="Register"/> again with the updated list.
+/// </remarks>
 public class CommandHookModSystem : ModSystem
 {
-    private readonly Dictionary<string, List<CommandRegistration>> _registrations = new();
+    internal static CommandHookModSystem? Instance;
+
+    private readonly List<ICommandHookListener> listeners = new();
+
+    private FrozenDictionary<string, FrozenDictionary<string, CommandRegistration>> registrations =
+        FrozenDictionary<string, FrozenDictionary<string, CommandRegistration>>.Empty;
+
+    private Harmony? harmony;
 
     public override bool ShouldLoad(EnumAppSide forSide) => forSide == EnumAppSide.Server;
 
+    public override double ExecuteOrder() => 0.0;
+
     public override void StartServerSide(ICoreServerAPI api)
     {
-        Mod.Logger.Notification("[CommandHook] Loaded");
+        Instance = this;
+        harmony = new Harmony(Mod.Info.ModID);
+        harmony.PatchAll();
+
+        Mod.Logger.Notification("Loaded");
     }
 
-    public void Register(string modId, CommandRegistration registration)
+    public override void Dispose()
     {
-        if (!_registrations.TryGetValue(modId, out var list))
+        harmony?.UnpatchAll(Mod.Info.ModID);
+        listeners.Clear();
+        Instance = null;
+    }
+
+    /// <summary>
+    /// Registers a listener for the commands it returns from <see cref="ICommandHookListener.Commands"/>.
+    /// </summary>
+    /// <remarks>
+    /// Calling this again for a <see cref="ICommandHookListener.ModId"/> that is already
+    /// registered replaces its previous registration. If the new <see cref="ICommandHookListener.Commands"/>
+    /// list is identical to the existing one this is a no-op, no rebuild happens.
+    /// If <paramref name="listener"/> has a null or empty <see cref="ICommandHookListener.Commands"/>
+    /// list, this just calls <see cref="Unregister"/> instead.
+    /// </remarks>
+    /// <param name="listener">The listener to register or update.</param>
+    public void Register(ICommandHookListener listener)
     {
-            list = new List<CommandRegistration>();
+        var commands = listener.Commands;
-            _registrations[modId] = list;
+
+        if (commands == null || commands.Count == 0)
+        {
+            Unregister(listener);
+            return;
         }
-        list.Add(registration);
+
+        int index = listeners.FindIndex(l => l.ModId == listener.ModId);
+
+        bool changed = false;
+        if (index < 0)
+        {
+            listeners.Add(listener);
+            changed = true;
+        }
+        else if (!CommandListEquals(listeners[index].Commands, commands))
+        {
+            listeners[index] = listener;
+            changed = true;
+        }
+
+        if (changed)
+            Rebuild();
+    }
+
+    /// <summary>
+    /// Removes a listener's registration entirely. Safe to call on a listener
+    /// that was never registered, this is a no-op in that case.
+    /// </summary>
+    /// <param name="listener">The listener to remove, matched by <see cref="ICommandHookListener.ModId"/>.</param>
+    public void Unregister(ICommandHookListener listener)
+    {
+        int index = listeners.FindIndex(l => l.ModId == listener.ModId);
+
+        if (index < 0)
+            return;
+
+        listeners.RemoveAt(index);
+        Rebuild();
+    }
+
+    // Rebuilds the dispatch table from listeners. Runs unconditionally and
+    // immediately whenever Register/Unregister actually changes something,
+    // no batching or startup-phase awareness. Command list sizes here are
+    // small (a handful of mods, a handful of commands each), so the realloc
+    // on every change is cheap and not worth the complexity of an incremental
+    // update. The two-level structure (command -> ModId -> registration) is
+    // what FireBefore/FireAfter walk on the hot path, so it's built once here
+    // instead of being derived per-dispatch.
+    private void Rebuild()
+    {
+        var builder = new Dictionary<string, Dictionary<string, CommandRegistration>>();
+
+        foreach (var listener in listeners)
+        {
+            var commands = listener.Commands;
+            if (commands == null || commands.Count == 0)
+                continue;
+
+            foreach (var cmd in commands)
+            {
+                if (!builder.TryGetValue(cmd, out var mods))
+                {
+                    mods = new Dictionary<string, CommandRegistration>();
+                    builder[cmd] = mods;
+                }
+                mods[listener.ModId] = listener.Registration;
+            }
+        }
+
+        var pruned = new Dictionary<string, FrozenDictionary<string, CommandRegistration>>(
+            builder.Count
+        );
+        foreach (var (cmd, mods) in builder)
+            if (mods.Count > 0)
+                pruned[cmd] = mods.ToFrozenDictionary();
+
+        registrations = pruned.ToFrozenDictionary();
+    }
+
+    // Called from ChatCommandApiPatch.Prefix before the real command executes.
+    // callingArgs is the engine's own live object, passed straight through,
+    // no allocation here. Each listener's Before is isolated in its own
+    // try/catch so one mod throwing doesn't stop the rest from running or
+    // break command dispatch for the server. The first listener to return a
+    // non-null result wins, we stop walking immediately rather than letting
+    // later listeners run against an already-cancelled command.
+    internal TextCommandResult? FireBefore(string commandName, TextCommandCallingArgs callingArgs)
+    {
+        if (registrations.TryGetValue(commandName, out var mods))
+        {
+            foreach (var (modId, reg) in mods)
+            {
+                TextCommandResult? result = null;
+                try
+                {
+                    result = reg.Before?.Invoke(callingArgs);
+                }
+                catch (Exception ex)
+                {
+                    Mod.Logger.Error("[{0}] Before /{1} threw: {2}", modId, commandName, ex);
+                }
+
+                if (result != null)
+                    return result;
+            }
+        }
+
+        return null;
+    }
+
+    // Called from the wrapped onCommandComplete in ChatCommandApiPatch.Prefix,
+    // only on the path where the command actually ran (no Before listener
+    // cancelled it). Same per-listener try/catch as FireBefore.
+    internal void FireAfter(
+        string commandName,
+        TextCommandCallingArgs callingArgs,
+        TextCommandResult result
+    )
+    {
+        if (registrations.TryGetValue(commandName, out var mods))
+        {
+            foreach (var (modId, reg) in mods)
+            {
+                try
+                {
+                    reg.After?.Invoke(callingArgs, result);
+                }
+                catch (Exception ex)
+                {
+                    Mod.Logger.Error("[{0}] After /{1} threw: {2}", modId, commandName, ex);
+                }
+            }
+        }
+    }
+
+    // Order-sensitive on purpose. Register only treats an update as a no-op
+    // if the list is identical in order and content, a reordered list still
+    // triggers a rebuild. Cheap to check (just an index walk, no allocation)
+    // and correctness here matters more than being lenient about ordering.
+    private static bool CommandListEquals(IReadOnlyList<string> a, IReadOnlyList<string> b)
+    {
+        if (a.Count != b.Count)
+            return false;
+
+        for (int i = 0; i < a.Count; i++)
+            if (a[i] != b[i])
+                return false;
+
+        return true;
     }
 }

CommandHook/CommandRegistration.cs

@@ -1,18 +1,28 @@
 namespace CommandHook;
 
+/// <summary>
+/// The pair of callbacks a listener provides for a command. Either one can be null.
+/// </summary>
 public struct CommandRegistration
 {
-    public readonly string CommandFilter;
+    /// <summary>
-    public readonly CommandHookDelegate? Before;
+    /// Called before the command runs. Return a non-null <see cref="TextCommandResult"/>
-    public readonly CommandHookDelegate? After;
+    /// to cancel the command, report that result to the caller, and skip any
+    /// remaining Before listeners. Return null to let the command run.
+    /// </summary>
+    public readonly CommandBeforeDelegate? Before;
 
-    public CommandRegistration(
+    /// <summary>
-        string commandFilter,
+    /// Called after the command runs, with its result. Not called if a Before
-        CommandHookDelegate? before,
+    /// listener cancelled the command.
-        CommandHookDelegate? after
+    /// </summary>
-    )
+    public readonly CommandAfterDelegate? After;
+
+    /// <summary>
+    /// Creates a registration. Pass null for either delegate to only hook one side.
+    /// </summary>
+    public CommandRegistration(CommandBeforeDelegate? before, CommandAfterDelegate? after)
     {
-        CommandFilter = commandFilter;
         Before = before;
         After = after;
     }

CommandHook/ICommandHookListener.cs

@@ -0,0 +1,28 @@
+using System.Collections.Generic;
+
+namespace CommandHook;
+
+/// <summary>
+/// Implement this to watch chat commands through CommandHook. Pass your
+/// implementation to <see cref="CommandHookModSystem.Register"/>.
+/// </summary>
+public interface ICommandHookListener
+{
+    /// <summary>
+    /// Your mod's id. Used as the namespace key so two mods can register
+    /// for the same command without colliding, and to find your existing
+    /// registration when you call <see cref="CommandHookModSystem.Register"/> again.
+    /// </summary>
+    string ModId { get; }
+
+    /// <summary>
+    /// The command names to watch, without the leading slash (e.g. "tp", not "/tp").
+    /// Return null or an empty list to stop watching anything.
+    /// </summary>
+    IReadOnlyList<string> Commands { get; }
+
+    /// <summary>
+    /// The Before/After delegates to invoke for the commands in <see cref="Commands"/>.
+    /// </summary>
+    CommandRegistration Registration { get; }
+}

CommandHook/assets/commandhook/lang/en.json

@@ -1,3 +1 @@
-{
+{}
-    "hello": "hello world!"
-}

CommandHook/modinfo.json

@@ -2,9 +2,11 @@
     "type": "code",
     "modid": "commandhook",
     "name": "CommandHook",
-    "description": "Exposes server command events for other mods to hook into.",
+    "description": "Adds before/after hooks for chat commands, for other mods to use.",
-    "authors": ["anth64"],
+    "authors": [
-    "version": "0.1.0",
+        "anth64"
+    ],
+    "version": "2.0.0",
     "side": "Server",
     "dependencies": {
         "game": "1.22.3"

README.md

@@ -0,0 +1,110 @@
+# CommandHook
+
+Server-side mod for Vintage Story. Lets other mods hook into chat commands
+before and after they run, namespaced per mod so multiple mods can watch
+the same command without colliding.
+
+## Usage
+
+Install it like any other mod. It has no effect unless something else
+depends on it.
+
+## For mod development
+
+Add it as a dependency in your `modinfo.json`:
+
+```json
+"dependencies": {
+    "game": "1.22.3",
+    "commandhook": "2.0.0"
+}
+```
+
+CommandHook patches `ChatCommandApi.Execute` and dispatches to whoever
+has registered. Your mod doesn't patch anything itself, it just implements
+`ICommandHookListener` and calls `CommandHookModSystem.Register`.
+
+A listener provides:
+
+- `ModId`, your mod's id. Used to namespace your registration so it doesn't
+  collide with another mod watching the same command
+- `Commands`, the command names you want to watch, without the leading slash
+- `Registration`, a `Before` delegate, an `After` delegate, or both
+
+`Before` receives the engine's own `TextCommandCallingArgs` for the
+invocation, live and unmodified. Return a non-null `TextCommandResult` to
+cancel the command and report that result to the caller, use the engine's
+own factories, e.g. `TextCommandResult.Error(...)` for a visible reason, or
+`TextCommandResult.Deferred` to cancel silently (per its own doc comment,
+this prints no output). Return `null` to let the command run normally. If
+you cancel, any remaining `Before` listeners for that command are skipped,
+and `After` never fires for that invocation.
+
+`After` receives the same `TextCommandCallingArgs` object plus the
+`TextCommandResult` the command actually produced. It only fires on the
+path where the command ran.
+
+### A note on `TextCommandCallingArgs` at Before vs After
+
+This is the engine's own live object, not a copy, what's safe to read
+depends on timing and on how the specific command you're watching is
+implemented internally.
+
+### Example
+
+```csharp
+public class MyListener : ICommandHookListener
+{
+    // Your mod's id, used to namespace this registration.
+    public string ModId => "mymod";
+
+    // Commands you want to watch, no leading slash.
+    public IReadOnlyList<string> Commands => new[] { "tp" };
+
+    // Wire up Before, After, or both.
+    public CommandRegistration Registration => new(Before, After);
+
+    private TextCommandResult? Before(TextCommandCallingArgs args)
+    {
+        // args.Caller.Player is null for console invocations.
+        if (args.Caller.Player is IServerPlayer player && !IsAllowed(player))
+        {
+            // Cancels the command, skips any remaining Before listeners,
+            // and After never fires for this invocation. The caller sees
+            // this exact result.
+            return TextCommandResult.Error("You're not allowed to do that", "notallowed");
+        }
+
+        return null;
+    }
+
+    private void After(TextCommandCallingArgs args, TextCommandResult result)
+    {
+        // Only runs if nothing cancelled. result is whatever the command
+        // actually produced, check result.Status for success/error/deferred.
+        if (result.Status != EnumCommandStatus.Success)
+            Logger.Warn($"A watched command failed: {result.StatusMessage}");
+    }
+}
+```
+
+Register it once your mod starts, unregister it on dispose:
+
+```csharp
+public override void StartServerSide(ICoreServerAPI api)
+{
+    // CommandHookModSystem.Instance is null if CommandHook isn't loaded,
+    // hence the ?. Register your listener once, here.
+    CommandHookModSystem.Instance?.Register(myListener);
+}
+
+public override void Dispose()
+{
+    // Always unregister on dispose, otherwise a stale listener stays in
+    // the dispatch table after your mod is gone.
+    CommandHookModSystem.Instance?.Unregister(myListener);
+}
+```
+
+Calling `Register` again with the same `ModId` replaces your existing
+registration. If your `Commands` list hasn't changed, it's a no-op.