docs: restructure README into Usage and For mod development sections

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,101 @@
+using System;
+using HarmonyLib;
+using Vintagestory.API.Common;
+using Vintagestory.API.Server;
+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 sets <see cref="CommandData.Cancel"/>, this returns
+    /// false to stop the original <c>Execute</c> from running at all, and
+    /// invokes <paramref name="onCommandComplete"/> directly with a
+    /// <see cref="EnumCommandStatus.Deferred"/> result so the caller doesn't see
+    /// a generic failure. 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, used here to resolve the sender.</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 sender = args.Caller.Player as IServerPlayer;
+        // Console invocations have no IServerPlayer, the cast just yields null,
+        // which is exactly what CommandData(sender, ...) expects to mean "console".
+        var data = new CommandData(sender, commandName, "/" + commandName);
+
+        if (system.FireBefore(commandName, ref data))
+        {
+            // Cancelled by a Before listener. Report Deferred rather than just
+            // swallowing it silently, so the original caller (console or player)
+            // sees something other than the command quietly doing nothing.
+            onCommandComplete?.Invoke(
+                new TextCommandResult { Status = EnumCommandStatus.Deferred }
+            );
+            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);
+            // data is captured by the closure, copy it so FireAfter gets its
+            // own ref target instead of sharing one with whatever else might
+            // still be holding the original local.
+            var afterData = data;
+            system.FireAfter(commandName, ref afterData, result);
+        };
+
+        return true;
+    }
+}

CommandHook/CommandData.cs

@@ -2,28 +2,59 @@ using Vintagestory.API.Server;
 
 namespace CommandHook;
 
+/// <summary>
+/// The data passed to Before/After listeners for a single command invocation.
+/// Passed by ref through the hot path, no allocation per command.
+/// </summary>
 public struct CommandData
 {
+    /// <summary>
+    /// The player who ran the command, or null if it came from the server console.
+    /// </summary>
     public readonly IServerPlayer? Sender;
+
+    /// <summary>
+    /// The full command text as typed, including the leading slash.
+    /// </summary>
     public readonly string FullCommand;
+
+    /// <summary>
+    /// The command name only, without the leading slash (matches what listeners
+    /// register in <see cref="ICommandHookListener.Commands"/>).
+    /// </summary>
     public readonly string CommandName;
     private byte flags;
 
     private const byte FlagIsPlayerCommand = 1 << 0;
     private const byte FlagCancel = 1 << 1;
 
+    /// <summary>
+    /// True if a player ran this command, false if it came from the server console.
+    /// Equivalent to <c>Sender != null</c>.
+    /// </summary>
     public bool IsPlayerCommand => (flags & FlagIsPlayerCommand) != 0;
+
+    /// <summary>
+    /// Set this to true in a Before listener to stop the command from executing.
+    /// Stops any remaining Before listeners from running too, and skips After entirely.
+    /// </summary>
     public bool Cancel
     {
         get => (flags & FlagCancel) != 0;
         set => flags = value ? (byte)(flags | FlagCancel) : (byte)(flags & ~FlagCancel);
     }
 
-    public CommandData(IServerPlayer? sender, string fullCommand)
+    /// <summary>
+    /// Creates the command data for a single invocation.
+    /// </summary>
+    /// <param name="sender">The player who ran the command, or null for console.</param>
+    /// <param name="commandName">The command name without the leading slash.</param>
+    /// <param name="fullCommand">The full command text as typed, including the leading slash.</param>
+    public CommandData(IServerPlayer? sender, string commandName, string fullCommand)
     {
         Sender = sender;
-        FullCommand = fullCommand.Length > 0 ? fullCommand.Trim() : string.Empty;
-        CommandName = FullCommand.Length > 1 ? FullCommand[1..].Split(' ')[0] : string.Empty;
+        CommandName = commandName;
+        FullCommand = fullCommand;
         flags = sender != null ? FlagIsPlayerCommand : (byte)0;
     }
 }

CommandHook/CommandDelegate.cs

@@ -0,0 +1,17 @@
+using Vintagestory.API.Common;
+
+namespace CommandHook;
+
+/// <summary>
+/// Invoked before a watched command runs. Set <c>data.Cancel = true</c> to stop
+/// the command from executing.
+/// </summary>
+/// <param name="data">The command data, passed by ref so you can read or cancel it.</param>
+public delegate void CommandBeforeDelegate(ref CommandData data);
+
+/// <summary>
+/// Invoked after a watched command has run. Not invoked if a Before listener cancelled it.
+/// </summary>
+/// <param name="data">The command data, passed by ref for consistency with <see cref="CommandBeforeDelegate"/>.</param>
+/// <param name="result">The result the command produced.</param>
+public delegate void CommandAfterDelegate(ref CommandData data, TextCommandResult result);

CommandHook/CommandHookDelegate.cs

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

CommandHook/CommandHookModSystem.cs

@@ -1,67 +1,217 @@
+using System;
+using System.Collections.Frozen;
 using System.Collections.Generic;
+using HarmonyLib;
 using Vintagestory.API.Common;
-using Vintagestory.API.Datastructures;
 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, CommandRegistration> wildcards = new();
-    private readonly Dictionary<string, ModRegistration> 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)
     {
+        Instance = this;
+        harmony = new Harmony(Mod.Info.ModID);
+        harmony.PatchAll();
+
         Mod.Logger.Notification("Loaded");
-        api.Event.PlayerChat += OnPlayerChat;
     }
 
-    public void RegisterWildcard(string modId, CommandRegistration registration)
+    public override void Dispose()
     {
-        wildcards[modId] = registration;
+        harmony?.UnpatchAll(Mod.Info.ModID);
+        listeners.Clear();
+        Instance = null;
     }
 
-    public void Register(string modId, string command, CommandRegistration registration)
+    /// <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)
     {
-        if (!registrations.TryGetValue(modId, out var modReg))
+        var commands = listener.Commands;
+
+        if (commands == null || commands.Count == 0)
         {
-            modReg = new ModRegistration(new Dictionary<string, CommandRegistration>());
-            registrations[modId] = modReg;
+            Unregister(listener);
+            return;
         }
 
-        modReg.Commands[command] = 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();
     }
 
-    private void OnPlayerChat(
-        IServerPlayer player,
-        int channelId,
-        ref string message,
-        ref string data,
-        BoolRef consumed
-    )
+    /// <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)
     {
-        if (!message.StartsWith('/'))
+        int index = listeners.FindIndex(l => l.ModId == listener.ModId);
+
+        if (index < 0)
             return;
 
-        var cmdData = new CommandData(player, message);
+        listeners.RemoveAt(index);
+        Rebuild();
+    }
 
-        foreach (var (_, reg) in wildcards)
-            reg.Before?.Invoke(ref cmdData);
+    // 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 (_, modReg) in registrations)
-            if (modReg.Commands.TryGetValue(cmdData.CommandName, out var reg))
-                reg.Before?.Invoke(ref cmdData);
+        foreach (var listener in listeners)
+        {
+            var commands = listener.Commands;
+            if (commands == null || commands.Count == 0)
+                continue;
 
-        if (cmdData.Cancel)
-            consumed.value = true;
+            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;
+            }
+        }
 
-        foreach (var (_, reg) in wildcards)
-            reg.After?.Invoke(ref cmdData);
+        var pruned = new Dictionary<string, FrozenDictionary<string, CommandRegistration>>(
+            builder.Count
+        );
+        foreach (var (cmd, mods) in builder)
+            if (mods.Count > 0)
+                pruned[cmd] = mods.ToFrozenDictionary();
 
-        foreach (var (_, modReg) in registrations)
-            if (modReg.Commands.TryGetValue(cmdData.CommandName, out var reg))
-                reg.After?.Invoke(ref cmdData);
+        registrations = pruned.ToFrozenDictionary();
+    }
+
+    // Called from ChatCommandApiPatch.Prefix before the real command executes.
+    // CommandData is passed by ref the whole way down, 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. If a listener sets data.Cancel, we stop walking the
+    // rest of the listeners immediately rather than letting them all run
+    // against an already-cancelled command.
+    internal bool FireBefore(string commandName, ref CommandData data)
+    {
+        if (registrations.TryGetValue(commandName, out var mods))
+        {
+            foreach (var (modId, reg) in mods)
+            {
+                try
+                {
+                    reg.Before?.Invoke(ref data);
+                }
+                catch (Exception ex)
+                {
+                    Mod.Logger.Error("[{0}] Before /{1} threw: {2}", modId, commandName, ex);
+                }
+
+                if (data.Cancel)
+                    break;
+            }
+        }
+
+        return data.Cancel;
+    }
+
+    // Called from the wrapped onCommandComplete in ChatCommandApiPatch.Prefix,
+    // only on the path where the command actually ran (Before didn't cancel).
+    // Same per-listener try/catch as FireBefore, but there's no early-out here
+    // since cancelling after the fact doesn't mean anything, the command
+    // already ran.
+    internal void FireAfter(string commandName, ref CommandData data, TextCommandResult result)
+    {
+        if (registrations.TryGetValue(commandName, out var mods))
+        {
+            foreach (var (modId, reg) in mods)
+            {
+                try
+                {
+                    reg.After?.Invoke(ref data, 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,11 +1,27 @@
 namespace CommandHook;
 
+/// <summary>
+/// The pair of callbacks a listener provides for a command. Either one can be null.
+/// </summary>
 public struct CommandRegistration
 {
-    public readonly CommandHookDelegate? Before;
-    public readonly CommandHookDelegate? After;
+    /// <summary>
+    /// Called before the command runs. Set <see cref="CommandData.Cancel"/> on
+    /// <paramref name="data"/> via the ref parameter inside your delegate to stop
+    /// the command from executing and skip any remaining Before listeners.
+    /// </summary>
+    public readonly CommandBeforeDelegate? Before;
 
-    public CommandRegistration(CommandHookDelegate? before, CommandHookDelegate? after)
+    /// <summary>
+    /// Called after the command runs, with its result. Not called if a Before
+    /// listener cancelled the command.
+    /// </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)
     {
         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/ModRegistration.cs

@@ -1,13 +0,0 @@
-using System.Collections.Generic;
-
-namespace CommandHook;
-
-public struct ModRegistration
-{
-    public readonly Dictionary<string, CommandRegistration> Commands;
-
-    public ModRegistration(Dictionary<string, CommandRegistration> commands)
-    {
-        Commands = commands;
-    }
-}

CommandHook/modinfo.json

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

README.md

@@ -0,0 +1,99 @@
+# 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": "1.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` runs before the command executes. Set `data.Cancel = true` inside it
+to stop the command from running. If you cancel, any remaining `Before`
+listeners for that command are skipped, and `After` never fires for that
+invocation.
+
+`After` runs once the command has produced a result. It only fires on the
+path where the command actually ran.
+
+`CommandData` is passed by `ref` the whole way through, no allocation per
+command.
+
+### 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 void Before(ref CommandData data)
+    {
+        // data.Sender is null for console invocations.
+        if (data.Sender != null && !IsAllowed(data.Sender))
+        {
+            // Stops the command from running and skips any remaining
+            // Before listeners. After never fires for this invocation.
+            data.Cancel = true;
+        }
+    }
+
+    private void After(ref CommandData data, 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($"/{data.CommandName} 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.