From 0d9ec655f7ce7381772ea7d46a3c05149b1ee584 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 16 Sep 2026 08:18:42 -0500 Subject: [PATCH 1/2] =?UTF-8?q?feat(bridge):=20protocol=202=20=E2=80=94=20?= =?UTF-8?q?the=20read=20path,=20and=20the=20first=20CI=20this=20repo=20has?= =?UTF-8?q?=20had?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifteen hooks: presence, deaths, chat, gathering, raided structures, bans, reports, login attempts and the wipe. Every frame now carries an envelope — `type`, `serverId` and `wipeId` — built in one place so nothing can emit a frame without one. Three rules the code enforces structurally rather than by intention: • A read-path hook never vetoes. Four of these are documented as "returning a non-null value overrides default behavior", so every hook is declared `void` and cannot answer. `CanUserLogin` is in the wave for what it observes. • A hook that can fire more than once a second per player is a counter. `OnDispenserGather` fires on every swing at a tree; it accumulates into a per-player tally flushed once a minute as one `player.tally` frame, as a delta rather than a running total. • `wipeId` is derived here, from the save's creation time, because this is the only component that can read it. PROTOCOL.md §8.2 reverses protocol 1 on that point deliberately. `server.hello` becomes a board rather than a greeting, and `players.online` joins it; both are re-sent on connect and every 60 seconds, which is what makes a restarted sidecar repopulate itself without asking. `rg.hooks` reports which hooks have actually fired. Hooks bind by name and arity through reflection on both frameworks, so a rename by Facepunch and a name Carbon's catalogue omits present identically — as silence. This is the standing answer to both, and it outranks either catalogue because it is a measurement. The repository had no `.gitea/workflows/` at all. `scripts/checkPlugin.js` asks the three questions a compiler here cannot: every hook is in `ExpectedHooks`, so `rg.hooks` can see it; every hook is `void`, unless answering is a decision written down in `ANSWERS_DELIBERATELY`; and `ProtocolVersion` agrees with `overlay.toml`, which is what stops a bundle that will not compose. The void rule is inverted on purpose — a list of *vetoable* hooks would have to be maintained against a catalogue in another repository, and the first one somebody forgot to add is the one that would pass. Its own suite breaks it seven ways, including the failure that would make the other six meaningless: a method parser that silently matches nothing. Proven on a live Oxide server: compiled, loaded, the envelope correct, both ban hooks firing, and the boards repopulating a sidecar whose database had been deleted 0.3 seconds earlier. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4 --- .gitea/workflows/pr-checks.yml | 61 ++ overlay.toml | 4 +- overlay/oxide/plugins/RunicGateway.cs | 949 +++++++++++++++++++++++++- scripts/checkPlugin.js | 176 +++++ scripts/checkPlugin.test.js | 130 ++++ 5 files changed, 1295 insertions(+), 25 deletions(-) create mode 100644 .gitea/workflows/pr-checks.yml create mode 100644 scripts/checkPlugin.js create mode 100644 scripts/checkPlugin.test.js diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml new file mode 100644 index 0000000..1fbfcda --- /dev/null +++ b/.gitea/workflows/pr-checks.yml @@ -0,0 +1,61 @@ +# Gate every pull request into `main`. +# +# This repository had no workflows at all — the same hole phase 2 found in +# Module-Rust, in the repo that ships the half running inside somebody's game +# server. It is the one component here that cannot be compiled by CI: the plugin +# is deployed as SOURCE and built by Oxide or Carbon against game assemblies that +# exist only on a Rust server, so a build job is not available at any price. +# +# What is available is a reader, and the mistakes worth reading for are the ones +# both frameworks make silent. Hooks bind by name and arity through reflection, +# with no compile-time check and no warning when a name matches nothing, so: +# +# • a hook that is not in `ExpectedHooks` is invisible to `rg.hooks`, which is +# the instrument this project relies on to answer "does this hook fire on +# this framework" (CARBON.md §6); +# • a hook that RETURNS something can cancel a death, swallow a player's +# gathered wood, or refuse a login (PROTOCOL.md §8.7); +# • a `ProtocolVersion` that disagrees with `overlay.toml` produces a bundle +# that will not compose, and the game link has no handshake to catch it. +# +# `scripts/checkPlugin.js` asks all three, dependency-free, and its own test +# suite breaks it seven ways — including the failure that would make every other +# case meaningless, a method parser that silently matches nothing. +# +# Enforcement (one-time, in the Gitea UI): +# Repository Settings → Branches → Branch Protection (rule for `main`) +# • Enable Status Check +# • Status check patterns: PR Checks / * +# Gitea only lists a context after it has reported once; the glob matches +# without the dropdown and keeps matching as jobs are added. + +name: PR Checks + +on: + pull_request: + branches: [main, edge] + +concurrency: + group: pr-checks-${{ github.ref }} + cancel-in-progress: true + +jobs: + plugin-checks: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 20 + + # No install step: the checks are dependency-free on purpose, which is also + # how a contributor runs them. + - name: Check the plugin's hooks, void rule and protocol declaration + run: node scripts/checkPlugin.js + + # Named individually rather than `node --test scripts/`: directory mode is + # not portable across the Node versions this project runs on. + - name: Test the checker itself + run: node --test scripts/checkPlugin.test.js diff --git a/overlay.toml b/overlay.toml index 3e12f77..d3a3ca7 100644 --- a/overlay.toml +++ b/overlay.toml @@ -26,8 +26,8 @@ # the same change that alters the emitters, exactly as the sidecar bumps # PROTOCOL_VERSION and the module bumps its own constant. # -# Current: 1 — the transport (docs/rust-link/PROTOCOL.md). -protocol = 1 +# Current: 2 — the transport plus the read path (docs/rust-link/PROTOCOL.md §8). +protocol = 2 # ── Oxide compatibility ────────────────────────────────────────────────────── # diff --git a/overlay/oxide/plugins/RunicGateway.cs b/overlay/oxide/plugins/RunicGateway.cs index fec886d..ef401a0 100644 --- a/overlay/oxide/plugins/RunicGateway.cs +++ b/overlay/oxide/plugins/RunicGateway.cs @@ -56,7 +56,7 @@ namespace Oxide.Plugins /// overlay's overlay.toml carries this same number so the installer can refuse /// to pair a sidecar and an overlay that disagree. /// - private const int ProtocolVersion = 1; + private const int ProtocolVersion = 2; /// /// Loopback reconnects are cheap, so the ceiling is low. A sidecar restart should cost @@ -67,6 +67,20 @@ namespace Oxide.Plugins /// The longest line accepted from the sidecar. Over-long lines are discarded. private const int MaxInboundLineBytes = 1024 * 1024; + /// + /// How often the boards are re-sent (PROTOCOL.md §8.3) and the per-player tallies are + /// flushed (§8.6). One cadence serves both because both answer the same question — + /// "what has this server been doing lately" — and two timers would only be two things + /// to get out of step. + /// + private const float BoardCadenceSeconds = 60f; + + /// + /// The side of one map grid cell, in world units. Rust's own grid, as every community + /// map and every server console uses it. + /// + private const float GridCellSize = 146.3f; + private static readonly ConcurrentQueue Outbound = new ConcurrentQueue(); private static readonly AutoResetEvent Wake = new AutoResetEvent(false); @@ -110,11 +124,92 @@ namespace Oxide.Plugins /// private string _bootId; + /// + /// The current wipe, as w-yyyyMMddTHHmmssZ, derived from the save's creation + /// time. Null until the server has saved at least once — a server with no save has no + /// wipe, and PROTOCOL.md §8.2 says that is reported by the field being **absent** + /// rather than by an empty string that will sort beside every other empty string for + /// ever. + /// + /// + /// Cached rather than read per frame, and re-read on OnNewSave, which is the + /// only moment it can change. A wipe is a fact about the world; nothing about a + /// reconnect, a plugin reload or a restart moves it. + /// + /// + private string _wipeId; + + /// + /// Per-player counters waiting to be flushed as a player.tally frame. Main + /// thread only — every writer is a game hook and the flush runs on a timer, which is + /// also the main thread, so this needs no lock and must never acquire one. + /// + private readonly Dictionary _tallies = new Dictionary(); + + /// + /// When each connected player connected, in epoch milliseconds, so a disconnect can + /// report a session length. Main thread only. + /// + /// + /// A player already on the server when this plugin loaded has no entry, and + /// sessionSec is then absent from their disconnect rather than zero: an + /// unknown session is not a zero-length one, and summing the zeros would under-report + /// playtime quietly and for ever. + /// + /// + private readonly Dictionary _connectedAt = new Dictionary(); + + /// + /// Every hook name this plugin declares, and whether it has fired at least once. + /// + /// + /// Both frameworks bind hooks by name and arity, through reflection, with no + /// compile-time check: a hook Facepunch renames on a Thursday simply stops being + /// called, and a hook one framework's catalogue does not list may or may not exist. + /// Neither failure logs anything anywhere. rg.hooks reads this dictionary, which + /// makes the plugin its own answer to both questions on whichever framework it is + /// actually running under — and a cheaper answer than either catalogue. + /// + /// + private readonly Dictionary _hookCounts = new Dictionary(); + + /// + /// The repeating boards/tally timer. + /// + /// + /// Typed as the plugin-facing Oxide.Plugins.Timer, which is what + /// timer.Every hands back — not Oxide.Core.Libraries.Timer.TimerInstance, + /// which is the library's own type and does not convert. Both frameworks would destroy + /// this timer for us when the plugin unloads; holding the handle is what lets + /// Unload stop it before the link thread is joined rather than after. + /// + /// + private Timer _cadence; + // ---- lifecycle ---- + /// + /// Every hook this plugin declares. Seeded into _hookCounts at zero so that + /// rg.hooks can report a hook that has never fired — which is the whole + /// question, and which a dictionary that only learns names as they arrive can never + /// answer. + /// + private static readonly string[] ExpectedHooks = + { + "OnServerInitialized", "OnServerShutdown", "OnNewSave", + "OnPlayerConnected", "OnPlayerDisconnected", "OnPlayerRespawned", + "OnPlayerDeath", "OnPlayerChat", "OnDispenserGather", "OnEntityDeath", + "OnPlayerReported", "OnUserBanned", "OnUserUnbanned", + "CanUserLogin", "OnUserApproved" + }; + private void Init() { + foreach (string hook in ExpectedHooks) + _hookCounts[hook] = 0L; + _bootId = ResolveBootId(); + _wipeId = ResolveWipeId(); _running = true; _link = new Thread(LinkLoop) @@ -128,6 +223,25 @@ namespace Oxide.Plugins private void Unload() { + // Best-effort, and it has to happen BEFORE `_running` goes false, because `Emit` + // refuses to enqueue once it does. A tally lost here is one interval of one player's + // gathering; flushing costs nothing and the ordering is the only reason it works at + // all. + try + { + FlushTallies(); + } + catch (Exception ex) + { + Puts($"could not flush tallies on unload: {ex.Message}"); + } + + if (_cadence != null) + { + _cadence.Destroy(); + _cadence = null; + } + _running = false; Wake.Set(); @@ -138,6 +252,7 @@ namespace Oxide.Plugins _link = null; _connected = false; + _tallies.Clear(); // A reload leaves the static queue holding lines addressed to a connection that no // longer exists. Clearing it is the difference between a reload and a slow leak. @@ -283,11 +398,11 @@ namespace Oxide.Plugins Interlocked.Increment(ref _connects); Puts($"connected to {host}:{port}"); - // Building the greeting reads the world, so it must happen on the main thread. - // It is sent on every connect rather than once at startup: the sidecar restarts - // independently of the game, and anything it needs up front has to be re-sent - // per connection. - Interface.Oxide.NextTick(SendHello); + // Building the boards reads the world, so it must happen on the main thread. + // They are sent on every connect rather than once at startup: the sidecar + // restarts independently of the game, and anything it needs up front has to be + // re-sent per connection. + Interface.Oxide.NextTick(SendBoards); NetworkStream localStream = stream; reader = new Thread(() => ReadLoop(localStream, epoch)) @@ -527,17 +642,14 @@ namespace Oxide.Plugins switch (verb) { case "ping": - EmitFrame(new Dictionary - { - { "kind", "pong" }, - { "t", NowMs() } - }); + // `control`: neither kept nor forwarded. It exists to move the sidecar's + // last-seen clock on a server nobody is playing on. + EmitFrame(Frame("pong", "control")); break; case "server.status": - Dictionary status = ServerStatus(); - status["kind"] = "server.status"; - status["t"] = NowMs(); + Dictionary status = Frame("server.status", "reply"); + Merge(status, ServerBody()); // The correlation id is echoed verbatim, and only when one was supplied. A // reply that invents one would be routed to nobody; a reply that omits one the @@ -556,13 +668,64 @@ namespace Oxide.Plugins // ---- frames ---- + /// + /// Builds the envelope every outbound frame shares (PROTOCOL.md §8.1). Nothing may + /// emit a frame it did not get from here. + /// + /// + /// is what the sidecar files on — event is appended to + /// history, snapshot replaces a board, reply is routed by + /// reqId, control is neither kept nor forwarded. It is the reason the + /// sidecar needs no arm per kind and the reason the catalogue can grow for free. + /// + /// + /// + /// wipeId is omitted when the server has never saved. A server with no + /// save has no wipe, and the difference between an absent field and an empty string is + /// the difference between a fact and a row that sorts beside every other empty string + /// for ever. + /// + /// + private Dictionary Frame(string kind, string type) + { + var frame = new Dictionary + { + { "kind", kind }, + { "type", type }, + { "t", NowMs() }, + { "serverId", _config != null ? _config.ServerId : "main" } + }; + + if (!string.IsNullOrEmpty(_wipeId)) + frame["wipeId"] = _wipeId; + + return frame; + } + + /// + /// Main thread. Both boards, in the order a reader wants them: what the server is, + /// then who is on it. + /// + /// + /// Sent on every connect and every cadence tick. A board is current state with exactly + /// one producer, re-sent rather than reconstructed — which is what makes "a restarted + /// sidecar is fully populated within one connection" true by construction instead of + /// by hoping nothing was in flight. + /// + /// + private void SendBoards() + { + SendHello(); + SendPlayersBoard(); + } + /// Main thread. private void SendHello() { - Dictionary hello = ServerStatus(); + Dictionary hello = Frame("server.hello", "snapshot"); + + Merge(hello, ServerBody()); - hello["kind"] = "server.hello"; - hello["t"] = NowMs(); hello["protocol"] = ProtocolVersion; hello["bootId"] = _bootId; hello["plugin"] = Version.ToString(); @@ -570,16 +733,52 @@ namespace Oxide.Plugins EmitFrame(hello); } + /// + /// Main thread. Who is connected right now, as the board the far side reconciles + /// against — the connect and disconnect events carry the story in between. + /// + private void SendPlayersBoard() + { + var players = new List(); + + if (BasePlayer.activePlayerList != null) + { + foreach (BasePlayer player in BasePlayer.activePlayerList) + { + if (player == null) + continue; + + var row = new Dictionary + { + { "steamId", player.UserIDString }, + { "name", player.displayName }, + { "sleeping", player.IsSleeping() } + }; + + long since; + if (_connectedAt.TryGetValue(player.userID, out since)) + row["connectedAt"] = since; + + players.Add(row); + } + } + + Dictionary frame = Frame("players.online", "snapshot"); + frame["count"] = players.Count; + frame["players"] = players; + + EmitFrame(frame); + } + /// /// Main thread. The shared body of server.hello and server.status — the /// two frames differ in what wraps them, not in what they say about the server, and /// letting them drift is how a site ends up showing two different player counts. /// - private Dictionary ServerStatus() + private Dictionary ServerBody() { var status = new Dictionary { - { "serverId", _config != null ? _config.ServerId : "main" }, { "hostname", ConVar.Server.hostname }, { "description", ConVar.Server.description }, { "level", ConVar.Server.level }, @@ -592,9 +791,9 @@ namespace Oxide.Plugins { "uptimeSec", (int)UnityEngine.Time.realtimeSinceStartup } }; - // The raw material a wipe id is derived from, rather than a wipe id. Deriving it is the - // website's job and it is not yet specified; emitting a guess now would bake a wrong - // one into stored rows, which is the expensive half to undo. + // The instant `wipeId` is derived from, kept in its own right: the id is for grouping + // rows and this is for showing a date, and deriving one back from the other is the + // kind of parsing nobody should have to do twice. try { status["saveCreatedAt"] = SaveRestore.SaveCreatedTime.ToUniversalTime() @@ -608,6 +807,664 @@ namespace Oxide.Plugins return status; } + /// + /// Puts an IP address onto a frame, or leaves it off. + /// + /// + /// The game does not always have one. A console banid against an offline id + /// produces the literal string "0" rather than an address or a null — + /// observed on a live server, not guessed — and a database column full of "0" is worse + /// than one full of nulls, because "0" survives every is-it-missing test a reader + /// writes and then fails whatever parses it. + /// + /// + /// + /// Absent, not zero. The same rule `saveCreatedAt` and `sessionSec` follow. + /// + /// + private static void PutAddress(Dictionary frame, string ip) + { + if (string.IsNullOrEmpty(ip) || ip == "0" || ip == "0.0.0.0") + return; + + frame["ip"] = ip; + } + + private static void Merge(Dictionary into, Dictionary from) + { + foreach (KeyValuePair pair in from) + into[pair.Key] = pair.Value; + } + + /// + /// Describes who or what killed somebody, onto . + /// + /// + /// Four outcomes, and collapsing any two of them loses something a killfeed needs: + /// player (a real kill, and the only one that belongs on a leaderboard), + /// self, npc and environment — falling, drowning, a helicopter's + /// fire, or the world simply deciding. `HitInfo` may legitimately be null, which is + /// the environment case and not an error. + /// + /// + private static void DescribeAttacker( + Dictionary frame, BasePlayer victim, HitInfo info) + { + BaseEntity initiator = info != null ? info.Initiator : null; + BasePlayer attacker = info != null ? info.InitiatorPlayer : null; + + string weapon = null; + + if (info != null) + { + if (info.WeaponPrefab != null) + weapon = info.WeaponPrefab.ShortPrefabName; + else if (info.Weapon != null) + weapon = info.Weapon.ShortPrefabName; + } + + if (weapon != null) + frame["weapon"] = weapon; + + if (initiator == null) + { + frame["attackerType"] = "environment"; + return; + } + + if (attacker != null && attacker.userID == victim.userID) + { + frame["attackerType"] = "self"; + return; + } + + if (attacker != null && !attacker.IsNpc) + { + frame["attackerType"] = "player"; + frame["attackerId"] = attacker.UserIDString; + frame["attackerName"] = attacker.displayName; + } + else + { + frame["attackerType"] = "npc"; + frame["attackerName"] = initiator.ShortPrefabName; + } + + float distance = info != null ? info.ProjectileDistance : 0f; + + if (distance <= 0f && initiator.transform != null && victim.transform != null) + distance = UnityEngine.Vector3.Distance( + initiator.transform.position, victim.transform.position); + + if (distance > 0f) + frame["distance"] = (float)Math.Round(distance, 1); + } + + // ---- the map grid ---- + + /// + /// An entity's position as Rust's own map reference (H7), or null if it has no + /// position to read. A reference, never a coordinate: a grid cell is 146.3 units + /// across, which is precise enough to say where a fight happened and far too coarse to + /// walk to somebody's door with. + /// + private static string GridOf(BaseEntity entity) + { + if (entity == null || entity.transform == null) + return null; + + return Grid(entity.transform.position); + } + + private static string Grid(UnityEngine.Vector3 position) + { + float size = ConVar.Server.worldsize; + + if (size <= 0f) + return null; + + float half = size / 2f; + int columns = (int)Math.Ceiling(size / GridCellSize); + + int x = (int)((position.x + half) / GridCellSize); + + // Rows are numbered from the NORTH edge downwards, which is why this is a subtraction + // and not the same expression as the column. Getting it wrong mirrors every grid on + // the map and is invisible without a second opinion. + int z = (int)((half - position.z) / GridCellSize); + + x = Clamp(x, 0, columns - 1); + z = Clamp(z, 0, columns - 1); + + return Column(x) + z.ToString(System.Globalization.CultureInfo.InvariantCulture); + } + + private static int Clamp(int value, int low, int high) + { + if (high < low) + return low; + + return value < low ? low : (value > high ? high : value); + } + + /// A column index as Rust spells it: 0 is A, 25 is Z, 26 is AA. + private static string Column(int index) + { + string name = string.Empty; + int n = index + 1; + + while (n > 0) + { + int remainder = (n - 1) % 26; + name = (char)('A' + remainder) + name; + n = (n - 1) / 26; + } + + return name; + } + + // ---- identity of the world ---- + + /// + /// The current wipe, as w-yyyyMMddTHHmmssZ, or null if this server has never + /// saved. + /// + /// + /// Protocol 1 left this to the website. Protocol 2 derives it here because by now + /// three components store rows that need it, and the plugin is the only one of the + /// three that can read the value — the other two would be re-deriving something they + /// were already told, and two derivations of one fact eventually disagree about a + /// boundary. + /// + /// + private static string ResolveWipeId() + { + try + { + DateTime created = SaveRestore.SaveCreatedTime.ToUniversalTime(); + + // A server that has never saved reports the bottom of the calendar. That is not a + // wipe in 0001, it is the absence of one. + if (created.Year < 2000) + return null; + + return "w-" + created.ToString( + "yyyyMMdd'T'HHmmss'Z'", System.Globalization.CultureInfo.InvariantCulture); + } + catch (Exception) + { + return null; + } + } + + /// + /// Records that a hook fired. See _hookCounts for why this exists at all. + /// + private void MarkHook(string name) + { + long count; + _hookCounts.TryGetValue(name, out count); + _hookCounts[name] = count + 1; + } + + // ---- the read path ---- + // + // Every hook below obeys three rules, and each of them has a failure behind it. + // + // 1. **A read-path hook never vetoes.** Four of these are documented by uMod as + // "returning a non-null value overrides default behavior" — a bridge that returned + // something by accident would cancel a death, swallow a player's wood or refuse a + // login, on somebody's production server at 3am. So they are declared `void`: both + // frameworks take the method's return value, and a void method has none. The rule is + // enforced by the signature rather than by remembering to write `return null`, which + // is the only version of it that survives a year of edits. + // + // 2. **A hook that can fire more than once a second per player is a counter, not an + // event.** `OnDispenserGather` fires on every swing at a tree. It accumulates into + // `_tallies` and leaves as one `player.tally` frame a minute (PROTOCOL.md §8.6). + // + // 3. **Every hook marks itself.** `MarkHook` is what makes `rg.hooks` able to say which + // of these actually fire on the framework this server is running — the standing + // answer both to Facepunch renaming a hook and to a framework catalogue that does not + // list one. + // + // All of it is the main thread, which is what makes reading `BasePlayer`, `ConVar` and a + // transform legal here and illegal in the reader thread. + + private void OnServerInitialized(bool initial) + { + MarkHook("OnServerInitialized"); + + // Started here rather than in Init: this hook fires both at the end of server startup + // and immediately on a hotload, so it is the one place that runs exactly once per + // load with the world available either way. + if (_cadence == null) + _cadence = timer.Every(BoardCadenceSeconds, Cadence); + + Dictionary frame = Frame("server.initialized", "event"); + frame["initial"] = initial; + EmitFrame(frame); + } + + private void OnServerShutdown() + { + MarkHook("OnServerShutdown"); + FlushTallies(); + EmitFrame(Frame("server.shutdown", "event")); + } + + /// + /// A new save is the only moment a wipe id can change, so it is the only place that + /// re-reads it. The frame carries both ids because "which wipe did this replace" is + /// the question a site asks when it draws the boundary, and it is unanswerable + /// afterwards. + /// + private void OnNewSave(string filename) + { + MarkHook("OnNewSave"); + + string previous = _wipeId; + _wipeId = ResolveWipeId(); + + // Built after the re-read, so `wipeId` on the envelope is the NEW wipe: this frame + // belongs to the world it opens, not to the one it closes. + Dictionary frame = Frame("server.wipe", "event"); + + if (!string.IsNullOrEmpty(previous)) + frame["previousWipeId"] = previous; + + EmitFrame(frame); + + // Nothing from the old world is still true. The tallies are the only such state this + // plugin holds; everything else lives on the far side of the link. + _tallies.Clear(); + _connectedAt.Clear(); + + Puts($"new save; wipe is now {(_wipeId ?? "unknown")}"); + } + + private void OnPlayerConnected(BasePlayer player) + { + MarkHook("OnPlayerConnected"); + + if (player == null) + return; + + _connectedAt[player.userID] = NowMs(); + + Dictionary frame = Frame("player.connected", "event"); + frame["steamId"] = player.UserIDString; + frame["name"] = player.displayName; + EmitFrame(frame); + } + + private void OnPlayerDisconnected(BasePlayer player, string reason) + { + MarkHook("OnPlayerDisconnected"); + + if (player == null) + return; + + // Before the frame: a session's last minute of gathering is worth more than the order + // of two lines, and after this point there is nothing to attribute it to. + FlushTally(player.userID); + + Dictionary frame = Frame("player.disconnected", "event"); + frame["steamId"] = player.UserIDString; + frame["name"] = player.displayName; + + if (!string.IsNullOrEmpty(reason)) + frame["reason"] = Flatten(reason); + + long since; + if (_connectedAt.TryGetValue(player.userID, out since)) + { + frame["sessionSec"] = (int)((NowMs() - since) / 1000L); + _connectedAt.Remove(player.userID); + } + + // A player who was already on when this plugin loaded has no recorded connect, so + // `sessionSec` is ABSENT rather than zero: a session of unknown length is not a + // session of no length, and summing zeros silently under-reports playtime for ever. + + EmitFrame(frame); + } + + private void OnPlayerRespawned(BasePlayer player) + { + MarkHook("OnPlayerRespawned"); + + if (player == null) + return; + + Dictionary frame = Frame("player.respawned", "event"); + frame["steamId"] = player.UserIDString; + frame["name"] = player.displayName; + EmitFrame(frame); + } + + /// + /// Void, and deliberately: uMod documents a non-null return here as overriding the + /// death itself. See rule 1 at the top of this region. + /// + private void OnPlayerDeath(BasePlayer player, HitInfo info) + { + MarkHook("OnPlayerDeath"); + + if (player == null || player.IsNpc) + return; + + Dictionary frame = Frame("player.death", "event"); + frame["steamId"] = player.UserIDString; + frame["name"] = player.displayName; + frame["sleeping"] = player.IsSleeping(); + + string grid = GridOf(player); + if (grid != null) + frame["grid"] = grid; + + DescribeAttacker(frame, player, info); + EmitFrame(frame); + } + + /// + /// Void, and deliberately: a non-null return here overrides chat delivery — the bridge + /// would be silently eating messages. + /// + private void OnPlayerChat(BasePlayer player, string message, ConVar.Chat.ChatChannel channel) + { + MarkHook("OnPlayerChat"); + + if (player == null || string.IsNullOrEmpty(message)) + return; + + Dictionary frame = Frame("player.chat", "event"); + frame["steamId"] = player.UserIDString; + frame["name"] = player.displayName; + frame["channel"] = channel.ToString(); + + // Flattened for the same reason every other string on this wire is: one frame is one + // line, and a chat message is the one field a player controls the bytes of. + frame["message"] = Flatten(message); + + EmitFrame(frame); + } + + /// + /// Void, and deliberately: a non-null return here replaces what the player receives. + /// This hook is a counter — see rule 2. + /// + private void OnDispenserGather(ResourceDispenser dispenser, BasePlayer player, Item item) + { + MarkHook("OnDispenserGather"); + + if (player == null || player.IsNpc || item == null || item.info == null) + return; + + TallyFor(player).Gather(item.info.shortname, item.amount); + } + + /// + /// Everything that dies which is not a player death: NPC kills, which are a counter, + /// and raided structures, which are an event with a `staff` class because a + /// structure's grid is where somebody lives. + /// + /// + /// The type checks are ordered cheapest-first and return early, because this hook + /// fires for every tree, barrel and animal on the map. + /// + /// + private void OnEntityDeath(BaseCombatEntity entity, HitInfo info) + { + MarkHook("OnEntityDeath"); + + if (entity == null) + return; + + BasePlayer attacker = info != null ? info.InitiatorPlayer : null; + bool byPlayer = attacker != null && !attacker.IsNpc; + + // Nothing below is interesting unless a real player did it. That single test discards + // decay, despawns and the world killing itself, which is the bulk of this hook. + if (!byPlayer) + return; + + BasePlayer victimPlayer = entity as BasePlayer; + + if (victimPlayer != null) + { + // A real player's death is OnPlayerDeath's, not ours — emitting here too would + // double every kill in the feed. Only the NPC kind is ours to count. + if (victimPlayer.IsNpc) + TallyFor(attacker).NpcKills++; + + return; + } + + if (entity is BaseNpc) + { + TallyFor(attacker).NpcKills++; + return; + } + + BuildingBlock block = entity as BuildingBlock; + + if (block == null || block.OwnerID == 0UL) + return; + + TallyFor(attacker).Structures++; + + Dictionary frame = Frame("entity.destroyed", "event"); + frame["ownerId"] = block.OwnerID.ToString(); + frame["prefab"] = block.ShortPrefabName; + frame["attackerId"] = attacker.UserIDString; + frame["attackerName"] = attacker.displayName; + + string grid = GridOf(block); + if (grid != null) + frame["grid"] = grid; + + EmitFrame(frame); + } + + private void OnPlayerReported( + BasePlayer reporter, string targetName, string targetId, + string subject, string message, string type) + { + MarkHook("OnPlayerReported"); + + Dictionary frame = Frame("player.reported", "event"); + + if (reporter != null) + { + frame["reporterId"] = reporter.UserIDString; + frame["reporterName"] = reporter.displayName; + } + + frame["targetId"] = targetId; + frame["targetName"] = targetName; + frame["subject"] = Flatten(subject); + frame["message"] = Flatten(message); + frame["reportType"] = type; + + EmitFrame(frame); + } + + private void OnUserBanned(string name, string id, string ipAddress, string reason) + { + MarkHook("OnUserBanned"); + + Dictionary frame = Frame("player.banned", "event"); + frame["steamId"] = id; + frame["name"] = name; + frame["reason"] = Flatten(reason); + PutAddress(frame, ipAddress); + EmitFrame(frame); + } + + private void OnUserUnbanned(string name, string id, string ipAddress) + { + MarkHook("OnUserUnbanned"); + + Dictionary frame = Frame("player.unbanned", "event"); + frame["steamId"] = id; + frame["name"] = name; + PutAddress(frame, ipAddress); + EmitFrame(frame); + } + + /// + /// Void, and this is the most important void in the file: a non-null return here + /// **kicks the player**, with the returned string as the reason. The bridge is here to + /// watch logins, never to decide one. + /// + /// + /// It fires on every connection attempt, approved or not — so it is an attempt, not a + /// denial. uMod publishes no OnUserRejected, and the only way to observe a + /// refusal from this hook is to be the thing refusing. A denial is therefore the + /// ABSENCE of a matching player.approved, decided by whoever reads the two + /// frames later; PROTOCOL.md §8.8 records that as a correction to the trigger + /// catalogue. + /// + /// + private void CanUserLogin(string name, string id, string ipAddress) + { + MarkHook("CanUserLogin"); + + Dictionary frame = Frame("player.login.attempt", "event"); + frame["steamId"] = id; + frame["name"] = name; + PutAddress(frame, ipAddress); + EmitFrame(frame); + } + + private void OnUserApproved(string name, string id, string ipAddress) + { + MarkHook("OnUserApproved"); + + Dictionary frame = Frame("player.approved", "event"); + frame["steamId"] = id; + frame["name"] = name; + PutAddress(frame, ipAddress); + EmitFrame(frame); + } + + // ---- tallies ---- + + /// + /// One player's counters between two flushes. Main thread only; no lock, ever. + /// + private class Tally + { + public readonly Dictionary Gathered = new Dictionary(); + public int NpcKills; + public int Structures; + public string Name; + + public void Gather(string shortname, int amount) + { + if (string.IsNullOrEmpty(shortname) || amount <= 0) + return; + + int have; + Gathered.TryGetValue(shortname, out have); + Gathered[shortname] = have + amount; + } + + public bool IsEmpty() + { + return Gathered.Count == 0 && NpcKills == 0 && Structures == 0; + } + } + + private Tally TallyFor(BasePlayer player) + { + Tally tally; + + if (!_tallies.TryGetValue(player.userID, out tally)) + { + tally = new Tally(); + _tallies[player.userID] = tally; + } + + tally.Name = player.displayName; + return tally; + } + + /// + /// Emits one player.tally per player with anything to report, and clears what it + /// emitted. + /// + /// + /// A tally frame is a delta, not a running total: it says what happened since + /// the last flush, so the far side sums rather than diffs. A missed frame then costs + /// one interval instead of corrupting the whole series, which is the failure mode a + /// running total has over a link that is allowed to drop frames (and this one is — + /// the outbound queue is drop-oldest by design). + /// + /// + private void FlushTallies() + { + if (_tallies.Count == 0) + return; + + // Materialised, because emitting mutates nothing here but clearing does, and iterating + // a dictionary while clearing it is a different bug on every runtime. + var ids = new List(_tallies.Keys); + + foreach (ulong id in ids) + FlushTally(id); + } + + private void FlushTally(ulong userId) + { + Tally tally; + + if (!_tallies.TryGetValue(userId, out tally)) + return; + + _tallies.Remove(userId); + + if (tally.IsEmpty()) + return; + + Dictionary frame = Frame("player.tally", "event"); + frame["steamId"] = userId.ToString(); + + if (!string.IsNullOrEmpty(tally.Name)) + frame["name"] = tally.Name; + + if (tally.Gathered.Count > 0) + frame["gathered"] = tally.Gathered; + + if (tally.NpcKills > 0) + frame["npcKills"] = tally.NpcKills; + + if (tally.Structures > 0) + frame["structures"] = tally.Structures; + + EmitFrame(frame); + } + + /// + /// The one repeating job: re-send the boards and flush the tallies. + /// + /// + /// It does nothing at all while the link is down. That is not an optimisation — the + /// outbound queue is drop-oldest, so a disconnected server would spend its whole + /// outage pushing minute-old boards over the top of the events that actually matter. + /// A reconnect re-sends the boards anyway, which is the whole point of a board. + /// + /// + private void Cadence() + { + if (!_connected) + return; + + FlushTallies(); + SendBoards(); + } + /// /// Collapses every run of whitespace and control characters to one space, so an /// exception message stays one log entry. @@ -703,7 +1560,53 @@ namespace Oxide.Plugins $"connected={_connected} depth={Volatile.Read(ref _depth)} " + $"sent={Interlocked.Read(ref _sent)} dropped={Interlocked.Read(ref _dropped)} " + $"received={Interlocked.Read(ref _received)} connects={Interlocked.Read(ref _connects)} " + - $"writeErrors={Interlocked.Read(ref _writeErrors)} bootId={_bootId}"); + $"writeErrors={Interlocked.Read(ref _writeErrors)} bootId={_bootId} " + + $"wipeId={(_wipeId ?? "none")}"); + } + + /// + /// Which of this plugin's hooks have actually fired, and how often. + /// + /// + /// Both frameworks bind hooks by name and arity through reflection, with no + /// compile-time check and no warning when a name matches nothing. So a hook Facepunch + /// renames on a Thursday, and a hook one framework's catalogue does not list, both + /// present identically: silence. This command is the standing answer to both, on + /// whichever framework the server is actually running — and it outranks either + /// catalogue, because it is a measurement rather than a document. + /// + /// + /// + /// A zero is not automatically a fault: OnNewSave fires on wipe day and + /// OnUserBanned only when somebody is banned. It is a fault when the thing it + /// names has demonstrably happened. + /// + /// + [ConsoleCommand("rg.hooks")] + private void CmdHooks(ConsoleSystem.Arg arg) + { + var fired = new List(); + var silent = new List(); + + foreach (string hook in ExpectedHooks) + { + long count; + _hookCounts.TryGetValue(hook, out count); + + if (count > 0L) + fired.Add($"{hook}={count}"); + else + silent.Add(hook); + } + + string firedText = fired.Count > 0 ? string.Join(" ", fired.ToArray()) : "(none)"; + string silentText = silent.Count > 0 ? string.Join(" ", silent.ToArray()) : "(none)"; + + arg.ReplyWith( + $"protocol={ProtocolVersion} hooks={ExpectedHooks.Length} " + + $"fired={fired.Count} silent={silent.Count}" + Environment.NewLine + + $"fired: {firedText}" + Environment.NewLine + + $"silent: {silentText}"); } } } diff --git a/scripts/checkPlugin.js b/scripts/checkPlugin.js new file mode 100644 index 0000000..56b04eb --- /dev/null +++ b/scripts/checkPlugin.js @@ -0,0 +1,176 @@ +#!/usr/bin/env node +// +// Static checks on the bridge plugin, run on every pull request. +// +// This plugin has no unit tests and cannot have any in the ordinary sense: it is +// deployed as SOURCE and compiled by Oxide or Carbon against game assemblies that +// exist only on a Rust server. There is no way to build it here, and the nearest +// thing to a compiler this repository owns is a reader. +// +// So these checks ask the questions a compiler would not answer anyway. Both +// frameworks bind hooks **by name and arity, through reflection**, with no +// compile-time check and no warning when a name matches nothing — which makes +// three mistakes silent, and each has a cost bigger than it looks: +// +// 1. A hook method the plugin declares but never lists in `ExpectedHooks`. +// `rg.hooks` is how we answer "does this hook fire on this framework" — +// the standing answer to Facepunch renaming one and to Carbon's catalogue +// omitting thirteen names (CARBON.md §6). A hook missing from that list is +// invisible to the one instrument built to see it. +// +// 2. A hook that ANSWERS. Four of the hooks in the read path are documented as +// "returning a non-null value overrides default behavior" — a bridge that +// returned something would cancel a death, swallow a player's gathered +// wood, or refuse a login, on somebody's production server at 3am +// (PROTOCOL.md §8.7). +// +// The rule is inverted on purpose: EVERY hook must be `void`, rather than +// every *vetoable* hook. A list of vetoable hook names would have to be +// maintained here, against a catalogue in another repository, and the first +// hook somebody forgot to add to it would be the one that passed. There is +// nothing to forget this way — a hook that must genuinely answer is added +// to `ANSWERS_DELIBERATELY` below, with a reason, as a visible exception. +// +// 3. A protocol version that disagrees with `overlay.toml`. The game link has +// no version handshake (PROTOCOL.md §2), so a half-bumped pair does not +// refuse — it mis-parses. `overlay.toml` exists precisely so the installer +// can refuse the pairing BEFORE an operator deploys it, and it is worth +// exactly as much as its agreement with the code. +// +// Dependency-free by design, like every check script in this project: it runs on +// a bare Node with no install step, which is also how a contributor runs it. +// +// node scripts/checkPlugin.js + +const fs = require('fs') +const path = require('path') + +const ROOT = path.resolve(__dirname, '..') +const PLUGIN = path.join(ROOT, 'overlay', 'oxide', 'plugins', 'RunicGateway.cs') +const OVERLAY_TOML = path.join(ROOT, 'overlay.toml') + +/** + * Hooks this plugin answers on purpose, and why. + * + * Empty, and it should stay empty for as long as the plugin is a read path. A + * name here is a deliberate decision to let the bridge change what the game + * does — reviewable because it is written down in one place rather than implied + * by a return type somewhere in 1,500 lines. + */ +const ANSWERS_DELIBERATELY = Object.create(null) + +/** Anything shaped like this is a game hook, by both frameworks' own convention. */ +const HOOK_NAME = /^(?:On|Can)[A-Z]\w*$/ + +/** + * Method declarations, as this file cares about them: the return type and the + * name. Deliberately narrow — it matches the plugin's own single style + * (`private [static] (`) rather than trying to parse C#. A method + * written some other way is not matched, which would let a hook through, so the + * shape is asserted by the self-test rather than assumed. + */ +const METHOD = /^\s*(?:private|public|protected|internal)\s+(?:static\s+)?([\w.<>[\],\s]+?)\s+(\w+)\s*\(/gm + +function readExpectedHooks(source) { + const block = /ExpectedHooks\s*=\s*\{([\s\S]*?)\}\s*;/.exec(source) + if (!block) return null + + return block[1] + .split(',') + .map((entry) => /"([^"]+)"/.exec(entry)) + .filter(Boolean) + .map((m) => m[1]) +} + +function readMethods(source) { + const found = [] + let m + METHOD.lastIndex = 0 + while ((m = METHOD.exec(source)) !== null) { + found.push({ returns: m[1].trim(), name: m[2] }) + } + return found +} + +function check(source, toml) { + const problems = [] + + const expected = readExpectedHooks(source) + if (!expected) { + return ['could not find the ExpectedHooks array in the plugin source'] + } + + const methods = readMethods(source) + const hooks = methods.filter((x) => HOOK_NAME.test(x.name)) + const hookNames = new Set(hooks.map((x) => x.name)) + + // 1. Every hook the plugin implements is one `rg.hooks` can report on. + for (const hook of hooks) { + if (!expected.includes(hook.name)) { + problems.push( + `${hook.name} is implemented but missing from ExpectedHooks, so rg.hooks cannot report it` + ) + } + } + + // 2. Every hook is void, unless answering is a decision somebody wrote down. + for (const hook of hooks) { + if (hook.returns === 'void') continue + if (hook.name in ANSWERS_DELIBERATELY) continue + + problems.push( + `${hook.name} returns ${hook.returns}, not void — a read-path hook must not be able to ` + + 'veto what the game was going to do (PROTOCOL.md §8.7). If it must answer, add it to ' + + 'ANSWERS_DELIBERATELY with a reason.' + ) + } + + // 3. No phantom entries: a name listed but never implemented reports "silent" + // for ever, which reads exactly like a hook the framework does not fire. + for (const name of expected) { + if (!hookNames.has(name)) { + problems.push(`ExpectedHooks lists ${name}, but no method of that name is implemented`) + } + } + + // 4. The two declaration sites this repository owns must agree. + const inCode = /ProtocolVersion\s*=\s*(\d+)\s*;/.exec(source) + const inToml = /^\s*protocol\s*=\s*(\d+)\s*$/m.exec(toml) + + if (!inCode) problems.push('could not read ProtocolVersion from the plugin source') + if (!inToml) problems.push('could not read `protocol` from overlay.toml') + + if (inCode && inToml && inCode[1] !== inToml[1]) { + problems.push( + `the plugin speaks protocol ${inCode[1]} and overlay.toml declares ${inToml[1]}. ` + + 'The installer refuses to pair a sidecar and an overlay that disagree, so a bundle built ' + + 'from this would not compose — and the game link itself has no version check to catch it.' + ) + } + + return problems +} + +function main() { + const source = fs.readFileSync(PLUGIN, 'utf8') + const toml = fs.readFileSync(OVERLAY_TOML, 'utf8') + + const problems = check(source, toml) + + if (problems.length > 0) { + console.error('The bridge plugin failed its static checks:\n') + for (const p of problems) console.error(` • ${p}`) + console.error('') + process.exit(1) + } + + const expected = readExpectedHooks(source) + const version = /ProtocolVersion\s*=\s*(\d+)\s*;/.exec(source)[1] + console.log( + `plugin ok — protocol ${version}, ${expected.length} hooks declared, every one void and listed` + ) +} + +module.exports = { check, readExpectedHooks, readMethods, HOOK_NAME } + +if (require.main === module) main() diff --git a/scripts/checkPlugin.test.js b/scripts/checkPlugin.test.js new file mode 100644 index 0000000..1e9cbbf --- /dev/null +++ b/scripts/checkPlugin.test.js @@ -0,0 +1,130 @@ +// A check is worth what it catches, so this breaks it seven ways. +// +// The case that matters most is the last one: `checkPlugin.js` finds hooks with a +// deliberately narrow regex, and a regex that silently matches NOTHING passes +// every check in this file and every check in CI while asserting nothing at all. +// So the real plugin source is read here too, and the parse is asserted against +// hooks that are known to be in it. +// +// node --test scripts/checkPlugin.test.js +// +// Named individually rather than `node --test scripts/`: directory mode is not +// portable across the Node versions this project runs on. + +const test = require('node:test') +const assert = require('node:assert') +const fs = require('node:fs') +const path = require('node:path') + +const { check, readExpectedHooks, readMethods, HOOK_NAME } = require('./checkPlugin') + +/** A minimal plugin that passes, as the baseline every case below deviates from. */ +function source({ expected = ['OnPlayerDeath'], methods, version = 2 } = {}) { + const body = + methods ?? + ` private void OnPlayerDeath(BasePlayer player, HitInfo info) + { + }` + + return `namespace Oxide.Plugins +{ + internal class RunicGateway : RustPlugin + { + private const int ProtocolVersion = ${version}; + + private static readonly string[] ExpectedHooks = + { + ${expected.map((e) => `"${e}"`).join(', ')} + }; + +${body} + } +}` +} + +const toml = (version = 2) => `protocol = ${version}\n` + +test('a plugin that follows the rules passes', () => { + assert.deepEqual(check(source(), toml()), []) +}) + +test('a hook missing from ExpectedHooks is caught, because rg.hooks could not report it', () => { + const problems = check(source({ expected: [] }), toml()) + assert.equal(problems.length, 1) + assert.match(problems[0], /OnPlayerDeath is implemented but missing from ExpectedHooks/) +}) + +test('a hook that can answer is caught — the rule the read path depends on', () => { + const methods = ` private object OnPlayerDeath(BasePlayer player, HitInfo info) + { + return null; + }` + + const problems = check(source({ methods }), toml()) + assert.equal(problems.length, 1) + assert.match(problems[0], /returns object, not void/) +}) + +test('returning null is not good enough — the signature is the rule', () => { + // `return null` today is one edit away from `return true` tomorrow, and the + // edit that breaks it looks harmless in a diff. A void method cannot be + // changed into a veto without changing its signature, which is visible. + const methods = ` private bool CanUserLogin(string name, string id, string ip) + { + return true; + }` + + const problems = check(source({ expected: ['CanUserLogin'], methods }), toml()) + assert.match(problems[0], /CanUserLogin returns bool, not void/) +}) + +test('a name listed but never implemented is caught, because it reports silent for ever', () => { + const problems = check(source({ expected: ['OnPlayerDeath', 'OnNewSave'] }), toml()) + assert.equal(problems.length, 1) + assert.match(problems[0], /ExpectedHooks lists OnNewSave, but no method/) +}) + +test('a protocol version that disagrees with overlay.toml is caught', () => { + const problems = check(source({ version: 3 }), toml(2)) + assert.equal(problems.length, 1) + assert.match(problems[0], /speaks protocol 3 and overlay\.toml declares 2/) +}) + +test('a method that is not shaped like a hook is left alone', () => { + // `Cadence`, `Frame`, `Flatten` and friends are ours, return real types, and + // must not be dragged into the void rule. + const methods = ` private Dictionary Frame(string kind, string type) + { + return null; + } + + private static string Column(int index) + { + return null; + }` + + assert.deepEqual(check(source({ expected: [], methods }), toml()), []) + assert.ok(!HOOK_NAME.test('Cadence')) + assert.ok(!HOOK_NAME.test('Frame')) + assert.ok(HOOK_NAME.test('OnPlayerDeath')) + assert.ok(HOOK_NAME.test('CanUserLogin')) +}) + +test('the parser actually reads the real plugin, rather than quietly matching nothing', () => { + const real = fs.readFileSync( + path.resolve(__dirname, '..', 'overlay', 'oxide', 'plugins', 'RunicGateway.cs'), + 'utf8' + ) + + const methods = readMethods(real) + const names = new Set(methods.map((m) => m.name)) + + // A narrow regex that matches nothing passes every other test in this file. + assert.ok(methods.length > 20, `only found ${methods.length} methods in the real plugin`) + for (const hook of ['OnPlayerDeath', 'OnPlayerConnected', 'CanUserLogin', 'OnNewSave']) { + assert.ok(names.has(hook), `${hook} was not found by the method parser`) + } + + const expected = readExpectedHooks(real) + assert.ok(expected.length >= 15, `only found ${expected.length} entries in ExpectedHooks`) +}) -- 2.49.1 From 551359c08e7ba1bb101cccbf265215c140a6a041 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 16 Sep 2026 08:29:24 -0500 Subject: [PATCH 2/2] fix(bridge): resolve the wipe at OnServerInitialized, and bound the connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects the Carbon rig found, neither visible on a development loopback. **The wipe id was null for every real session.** `Init` runs before the save is loaded, so `SaveRestore.SaveCreatedTime` is not yet meaningful there and the id resolved at load time stayed null for the life of the process — every frame shipped without the field R12 splits history on. It was invisible because a hot-reloaded plugin reads an already-loaded world and gets the right answer every time; it took a server that BOOTED with the plugin installed, which is every real one, to show `wipeId=none` beside a save sitting on disk. Resolved again at `OnServerInitialized`, and lazily while still unknown. **Unload blocked the main thread for two seconds against an unreachable sidecar.** Observed as `hook 'Unload' took longer than 100ms [2002ms]` next to `link thread did not stop cleanly` — phase 1's stall arriving by a different road. The link thread sits in a blocking `TcpClient.Connect`, which has no timeout of its own and cannot be woken; a host that REFUSES answers instantly, which is every loopback test, and a host that DROPS does not answer at all. The connect is now bounded, and waits on a stop handle of its own so a reload abandons it at once. It cannot share `Wake`, which also means 'the queue has something in it' and is signalled by every hook. After: the same reload logs no slow-hook warning and no stranded thread. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4 --- overlay/oxide/plugins/RunicGateway.cs | 110 ++++++++++++++++++++++++-- 1 file changed, 105 insertions(+), 5 deletions(-) diff --git a/overlay/oxide/plugins/RunicGateway.cs b/overlay/oxide/plugins/RunicGateway.cs index ef401a0..dd1fb84 100644 --- a/overlay/oxide/plugins/RunicGateway.cs +++ b/overlay/oxide/plugins/RunicGateway.cs @@ -67,6 +67,25 @@ namespace Oxide.Plugins /// The longest line accepted from the sidecar. Over-long lines are discarded. private const int MaxInboundLineBytes = 1024 * 1024; + /// + /// How long to wait for the TCP connect itself before giving up and backing off. + /// + /// + /// It exists because TcpClient.Connect is blocking and has no timeout of its + /// own, and the OS default is tens of seconds. A host that REFUSES answers instantly — + /// which is every loopback test, and why this was invisible for two phases — but a host + /// that drops does not answer at all: a firewall, a wrong address, a machine + /// that is off. The link thread then sits in the connect, deaf to everything, and + /// Unload waits out its whole join on the game's MAIN THREAD. + /// + /// + /// + /// Generous against any real network and short against a hang. Loopback, which is the + /// supported deployment, connects in well under a millisecond. + /// + /// + private const int ConnectTimeoutMs = 3000; + /// /// How often the boards are re-sent (PROTOCOL.md §8.3) and the per-player tallies are /// flushed (§8.6). One cadence serves both because both answer the same question — @@ -84,6 +103,18 @@ namespace Oxide.Plugins private static readonly ConcurrentQueue Outbound = new ConcurrentQueue(); private static readonly AutoResetEvent Wake = new AutoResetEvent(false); + /// + /// Set once, by Unload, and never reset. It is what lets a thread blocked on a + /// CONNECT give up immediately instead of at the end of its timeout. + /// + /// + /// It cannot be : that handle also means "the queue has something in + /// it", which Emit signals constantly, so waiting on it during a connect would + /// abandon the attempt every time a hook fired. Two meanings, two handles. + /// + /// + private readonly ManualResetEvent _stopping = new ManualResetEvent(false); + private PluginConfig _config; private Thread _link; @@ -223,10 +254,19 @@ namespace Oxide.Plugins private void Unload() { - // Best-effort, and it has to happen BEFORE `_running` goes false, because `Emit` - // refuses to enqueue once it does. A tally lost here is one interval of one player's - // gathering; flushing costs nothing and the ordering is the only reason it works at - // all. + // Attempted before `_running` goes false, because `Emit` refuses to enqueue once it + // does — but be honest about what this achieves: the writer stops on the same flag a + // few lines below and the queue is cleared after the join, so a frame enqueued here + // only survives if the writer happens to be mid-drain. It usually is not. + // + // That is left as it is rather than fixed with a drain-before-exit, because draining + // means waiting on a socket from `Unload`, which runs on the MAIN THREAD — the exact + // stall phase 1 removed when a 1.9-second reload showed up as + // `Calling 'Unload' took 1918ms`. A wedged peer would make it worse than the bug. + // + // What is actually lost is under a minute of one player's gathering, and only on a + // manual plugin reload: a real shutdown flushes at `OnServerShutdown`, and a player + // leaving flushes at their disconnect. try { FlushTallies(); @@ -243,6 +283,7 @@ namespace Oxide.Plugins } _running = false; + _stopping.Set(); Wake.Set(); Thread t = _link; @@ -385,7 +426,7 @@ namespace Oxide.Plugins client = new TcpClient(); client.NoDelay = true; - client.Connect(host, port); + Connect(client, host, port); NetworkStream stream = client.GetStream(); stream.WriteTimeout = 5000; // a wedged peer must surface as an error, not a hang @@ -479,6 +520,48 @@ namespace Oxide.Plugins _connected = false; } + /// + /// Connects with a bound, and with a way out. + /// + /// + /// The wait ends on whichever comes first: the connect completing, the plugin being + /// unloaded, or . Without the middle one a reload against + /// an unreachable sidecar costs the game's main thread the whole join timeout — + /// observed as hook 'Unload' took longer than 100ms [2002ms] beside + /// link thread did not stop cleanly, which is phase 1's stall arriving by a + /// different road. + /// + /// + private void Connect(TcpClient client, string host, int port) + { + IAsyncResult pending = client.BeginConnect(host, port, null, null); + + int signalled = WaitHandle.WaitAny( + new WaitHandle[] { pending.AsyncWaitHandle, _stopping }, ConnectTimeoutMs); + + if (signalled != 0) + { + // Closing the client is what abandons the in-flight connect; the socket would + // otherwise stay open behind an IAsyncResult nobody is going to end. + try + { + client.Close(); + } + catch + { + } + + throw new System.IO.IOException( + signalled == 1 + ? "unloading" + : $"connect to {host}:{port} timed out after {ConnectTimeoutMs}ms"); + } + + // Throws the real socket error — a refusal, an unknown host — which is what the caller + // logs. Only reached when the connect actually completed or failed on its own. + client.EndConnect(pending); + } + private void WriteLoop(NetworkStream stream) { while (_running && !_dead) @@ -696,6 +779,12 @@ namespace Oxide.Plugins { "serverId", _config != null ? _config.ServerId : "main" } }; + // Resolved lazily while it is still unknown, which covers the case above on a + // framework that never fires `OnServerInitialized` — and costs one static property + // read per frame only until the answer exists. + if (string.IsNullOrEmpty(_wipeId)) + _wipeId = ResolveWipeId(); + if (!string.IsNullOrEmpty(_wipeId)) frame["wipeId"] = _wipeId; @@ -1035,6 +1124,17 @@ namespace Oxide.Plugins { MarkHook("OnServerInitialized"); + // Re-read the wipe, and this is not belt-and-braces: at a real server + // start `Init` runs BEFORE the save is loaded, so `SaveCreatedTime` is not yet + // meaningful and the id resolved there is null for the entire session. This hook is + // the first moment the world is actually there. + // + // It was invisible on a development rig for the reason such things usually are: a + // hot-reloaded plugin reads an already-loaded world and gets the right answer every + // time. It took a server that BOOTED with the plugin installed — which is every real + // one — to show `wipeId=none` on a server with a save sitting on disk. + _wipeId = ResolveWipeId(); + // Started here rather than in Init: this hook fires both at the end of server startup // and immediately on a hotload, so it is the one place that runs exactly once per // load with the world available either way. -- 2.49.1