diff --git a/.github/workflows/hud-workshop.yaml b/.github/workflows/hud-workshop.yaml index b6f92a0e..206616fe 100644 --- a/.github/workflows/hud-workshop.yaml +++ b/.github/workflows/hud-workshop.yaml @@ -48,6 +48,7 @@ jobs: KEY=$( { git rev-parse "HEAD:$HUD/panorama" git hash-object "$HUD/build.sh" "$HUD/addoninfo.txt" ${PREVIEW:+"$PREVIEW"} echo "${PANORAMA_COMPILER_REF:-main}" + echo "${{ inputs.item_id }}" } | sha256sum | cut -c1-16 ) TAG="hud-published/$KEY" echo "key=$KEY" >> "$GITHUB_OUTPUT" @@ -91,6 +92,10 @@ jobs: # into .tools on Linux, so nothing here needs Windows or a CS2 install. - name: Build the addon if: steps.changed.outputs.proceed == 'yes' + # The vpk must be named after the item it will live in, so an update + # needs the id at BUILD time, not just at upload time. + env: + WORKSHOP_ITEM_ID: ${{ inputs.item_id }} run: ./apps/utility-sw/hud/build.sh # An addon that mounts and resolves nothing is the failure mode with no @@ -100,8 +105,20 @@ jobs: if: steps.changed.outputs.proceed == 'yes' run: | set -euo pipefail - VPK=apps/utility-sw/hud/build/upload/5stack_utility_hud.vpk - [ -f "$VPK" ] || { echo "::error::no vpk was produced"; exit 1; } + ITEM_ID="${{ inputs.item_id }}" + VPK=$(ls apps/utility-sw/hud/build/upload/*.vpk 2>/dev/null | head -1) + [ -n "$VPK" ] && [ -f "$VPK" ] || { echo "::error::no vpk was produced"; exit 1; } + + # An addon named anything but its id downloads and then fails to mount, + # and the mount error names a path rather than the mismatch. + if [ -n "$ITEM_ID" ] && [ "$(basename "$VPK")" != "$ITEM_ID.vpk" ]; then + echo "::error::packed $(basename "$VPK") but item $ITEM_ID needs $ITEM_ID.vpk" + exit 1 + fi + if [ -z "$ITEM_ID" ]; then + echo "::warning::no item_id given, so the vpk is not named after an id." + echo "::warning::Only correct for a first publish; re-run with item_id afterwards." + fi TREE=$(apps/utility-sw/hud/.tools/vpkeditcli --file-tree "$VPK" 2>&1 || true) echo "$TREE" diff --git a/apps/swiftly/Dockerfile b/apps/swiftly/Dockerfile index c6930365..eb2b505c 100644 --- a/apps/swiftly/Dockerfile +++ b/apps/swiftly/Dockerfile @@ -108,6 +108,17 @@ ENV METAMOD_CSGO_URL=https://mms.alliedmods.net/mmsdrop/1.12/mmsource-1.12.0-git ENV SOURCEMOD_CSGO_URL=https://sm.alliedmods.net/smdrop/1.13/sourcemod-1.13.0-git7297-linux.tar.gz ENV NO_LOBBY_RESERVATION_URL=https://github.com/nuxencs/NoLobbyReservation/releases/download/v0.0.1/NoLobbyReservation.zip +# The engine natively mounts exactly one addon -- the map -- so the HUD workshop +# item needs this to reach clients at all. Practice servers only; see setup.sh. +# Blank disables the mount: the plugin degrades to centre text without it. +ENV HUD_WORKSHOP_ID="3791548068" + +# ARG, not ENV: the download below happens once at build time, so these are set +# with --build-arg. As ENV they read as runtime knobs beside HUD_WORKSHOP_ID and +# would silently do nothing when overridden at docker run. +ARG ADDONS_MANAGER_VERSION="v2.0.3" +ARG ADDONS_MANAGER_URL=https://github.com/SwiftlyS2-Plugins/AddonsManager/releases/download/${ADDONS_MANAGER_VERSION}/AddonsManager.zip + RUN apt-get update && \ apt-get install -y --no-install-recommends \ wget locales procps jq ca-certificates curl unzip rsync \ @@ -128,6 +139,10 @@ RUN wget -q $SWIFTLYS2_URL -O /tmp/swiftlys2.zip && \ unzip -q /tmp/swiftlys2.zip -d /opt/swiftlys2 && \ rm /tmp/swiftlys2.zip +RUN wget -q $ADDONS_MANAGER_URL -O /tmp/addons-manager.zip && \ + unzip -q /tmp/addons-manager.zip -d /opt/addons-manager && \ + rm /tmp/addons-manager.zip + RUN mkdir -p /opt/metamod /opt/counterstrikesharp && \ wget -q $METAMOD_URL -O /tmp/metamod.tar.gz && \ tar -xz -C /opt/metamod -f /tmp/metamod.tar.gz && \ diff --git a/apps/swiftly/scripts/setup.sh b/apps/swiftly/scripts/setup.sh index e8aa451b..911e702a 100755 --- a/apps/swiftly/scripts/setup.sh +++ b/apps/swiftly/scripts/setup.sh @@ -204,6 +204,56 @@ if $INSTALL_UTILITY_PRACTICE_PLUGIN = true ; then else echo "---Utility Practice: plugin dir already present, skipping /opt/utility-practice symlink---" fi + + # The HUD's Panorama layouts live in a workshop addon, and CS2 has no + # server-to-client file transfer -- AddonsManager names the id to connecting + # clients and Steam delivers it. Without this the plugin still works, falling + # back to centre text, so a failure here must not stop the server. + # + # AddonsManager reads its config with optional:false and validates it on + # start, so anything that is not a bare numeric id writes JSON that takes the + # plugin down at load instead of degrading -- a pasted quote, a trailing + # comma, a second id. That is a typo, not a reason to lose the server. + case "${HUD_WORKSHOP_ID}" in + "") + echo "---Utility Practice: HUD_WORKSHOP_ID unset, HUD falls back to centre text---" + ;; + *[!0-9]*) + echo "---Utility Practice: HUD_WORKSHOP_ID '${HUD_WORKSHOP_ID}' is not a workshop id, HUD falls back to centre text---" + ;; + *) + ADDONS_MANAGER_PLUGIN_DIR="${INSTANCE_SERVER_DIR}/game/csgo/addons/swiftlys2/plugins/AddonsManager" + if [ ! -e "$ADDONS_MANAGER_PLUGIN_DIR" ]; then + ln -s "/opt/addons-manager/AddonsManager" "$ADDONS_MANAGER_PLUGIN_DIR" + fi + + # Rewritten every boot: the id is ours, and a stale copy from an older + # image would silently serve the wrong addon. + # + # RedownloadAddonOnMount matters because this addon changes -- unlike a + # map, which is published once. AddonsManager only checks that an item is + # installed, not that it is current, so without this a server that cached + # an older HUD keeps serving those layouts forever and never picks up a + # republish. + # + # Through materialize_for_write because configs is a symlink onto the + # node-wide volume: written straight through it, one practice server's + # addon id lands in front of every other server on the node -- and + # AddonsManager watches the file, so a running server swaps to it live. + ADDONS_MANAGER_CONFIG="$(materialize_for_write "${INSTANCE_SERVER_DIR}/game/csgo" "addons/swiftlys2/configs/plugins/AddonsManager/config.jsonc")" + cat > "$ADDONS_MANAGER_CONFIG" <.vpk`. + +**The vpk must be named after the workshop item id.** CS2 resolves an addon as +`steamapps/workshop/content/730//.vpk`, so a friendly name downloads +correctly and then fails to mount, and the mount error names a path rather than +the mismatch. Set `WORKSHOP_ITEM_ID` when building anything destined for the +Workshop; it falls back to `5stack_utility_hud` only for the very first publish, +before an id exists. The CI workflow passes `item_id` through for this and +refuses to upload a vpk whose name does not match. PanoramaCompiler writes Source 2 containers directly and never invokes `resourcecompiler.exe`, so this needs only the .NET 10 SDK — no Windows, no CS2 install. @@ -124,8 +132,14 @@ text that has always worked, and `.hud` lets a player choose the text path anyway. - **Development** — point CS2 at `build/` as a local override via `gameinfo.gi`. -- **Production** — publish `build/` to the Steam Workshop and name its id in - [AddonsManager](https://github.com/SwiftlyS2-Plugins/AddonsManager). +- **Production** — already wired. `HUD_WORKSHOP_ID` in `apps/swiftly/Dockerfile` + holds the published id, and `setup.sh` installs + [AddonsManager](https://github.com/SwiftlyS2-Plugins/AddonsManager) and writes + its config on practice servers only. Set the variable empty to disable the + mount; the plugin then falls back to centre text. It must be a bare numeric + id — anything else is refused with a log line and treated as unset, because + AddonsManager validates its config on start and would otherwise fail to load + rather than degrade. There is no way round the Workshop, and it is not a gap in SwiftlyS2. CS2 has no server-to-client file transfer at all — no `sv_downloadurl`, no file netmessage; @@ -255,5 +269,20 @@ App 730 does accept a SteamCMD workshop upload — verified on 2026-08-28, item its `VpkDirectories` whitelist are therefore avoidable entirely, which is the whole reason this path exists. -Practice servers first. A one-time addon download is a fair price for someone -who typed `.drill`; it is not something to ask ten people for before a match. +Practice servers first, which is why the install sits inside the +`INSTALL_UTILITY_PRACTICE_PLUGIN` branch of `setup.sh` rather than beside the +match plugin. A one-time addon download is a fair price for someone who typed +`.drill`; it is not something to ask ten people for before a match. + +Verify a published item is actually downloadable before relying on it — this is +exactly what a joining client does, and it fails for reasons the Workshop page +does not show: + +```sh +steamcmd +login anonymous +workshop_download_item 730 3791548068 +quit +``` + +A newly published item returns `Access Denied` for some minutes until Steam +finishes processing it, while the API already reports `result: 1` and +`visibility: 0`. That gap is not an error and not a review queue; it just +resolves on its own. diff --git a/apps/utility-sw/hud/build.sh b/apps/utility-sw/hud/build.sh index c2a4e504..91e248cc 100755 --- a/apps/utility-sw/hud/build.sh +++ b/apps/utility-sw/hud/build.sh @@ -21,7 +21,13 @@ DIST_DIR="${HUD_DIR}/dist" COMPILER_REPO="https://github.com/nicedayzhu/PanoramaCompiler.git" COMPILER_REF="${PANORAMA_COMPILER_REF:-main}" -ADDON_NAME="5stack_utility_hud" +# CS2 resolves a workshop addon by its published file id, not by a friendly +# name: AddonsManager (and the engine) look for +# steamapps/workshop/content/730//.vpk. A differently-named vpk +# downloads fine and then fails to mount, with the error pointing at a path that +# looks unrelated to the name you chose. WORKSHOP_ITEM_ID is unset only for the +# very first publish, before an id exists. +ADDON_NAME="${WORKSHOP_ITEM_ID:-5stack_utility_hud}" log() { printf '\033[36m==>\033[0m %s\n' "$1"; } die() { printf '\033[31mx\033[0m %s\n' "$1" >&2; exit 1; } diff --git a/apps/utility-sw/hud/publish.sh b/apps/utility-sw/hud/publish.sh index 702b1437..588612ec 100755 --- a/apps/utility-sw/hud/publish.sh +++ b/apps/utility-sw/hud/publish.sh @@ -39,6 +39,13 @@ fi [ -d "${CONTENT_DIR}" ] || die "build.sh produced no ${CONTENT_DIR} (is vpkeditcli installed?)" +# CS2 resolves an addon by its id: steamapps/workshop/content/730//.vpk. +# build.sh reads WORKSHOP_ITEM_ID for exactly this, so a mismatch here means the +# build ran without it -- the item would download and then fail to mount. +if [ "${PUBLISHED_FILE_ID}" != "0" ] && [ ! -f "${CONTENT_DIR}/${PUBLISHED_FILE_ID}.vpk" ]; then + die "expected ${CONTENT_DIR}/${PUBLISHED_FILE_ID}.vpk; re-run build.sh with WORKSHOP_ITEM_ID=${PUBLISHED_FILE_ID}" +fi + # An item with no vpk publishes fine and does nothing, which is the failure this # whole file exists to avoid. find "${CONTENT_DIR}" -name '*.vpk' | grep -q . || die "no .vpk in ${CONTENT_DIR}" diff --git a/apps/utility-sw/src/Hud/PracticeHud.cs b/apps/utility-sw/src/Hud/PracticeHud.cs index 9748d6b5..c00e1a6f 100644 --- a/apps/utility-sw/src/Hud/PracticeHud.cs +++ b/apps/utility-sw/src/Hud/PracticeHud.cs @@ -573,19 +573,16 @@ private List MenuRows(ulong steamId) } } - if (_reachableOnly.Contains(steamId)) - { - Vec3? at = PracticeSystem.Where(_system.Find(steamId))?.feet_position; + Vec3? standing = Standing(steamId); - if (at != null) - { - var here = PracticeReplay - .SpotAt(_library.For(steamId), at.Value) - .Select(UtilityTargetCluster.Key) - .ToHashSet(StringComparer.Ordinal); + if (_reachableOnly.Contains(steamId) && standing != null) + { + var here = PracticeReplay + .SpotAt(_library.For(steamId), standing.Value) + .Select(UtilityTargetCluster.Key) + .ToHashSet(StringComparer.Ordinal); - lineups = lineups.Where(l => here.Contains(UtilityTargetCluster.Key(l))); - } + lineups = lineups.Where(l => here.Contains(UtilityTargetCluster.Key(l))); } List matched = lineups @@ -599,8 +596,6 @@ private List MenuRows(ulong steamId) ) .ToList(); - Vec3? standing = PracticeSystem.Where(_system.Find(steamId))?.feet_position; - if (standing == null) { return matched; @@ -692,7 +687,20 @@ private bool RenderMenu(IPlayer player) } } - return _reachableOnly.Contains(steamId) ? "Throwable from here" : null; + // Only while it is actually narrowing anything. A player who is dead or + // between pawns has nowhere to be throwing from, so the filter stands + // down -- and a full list under a heading that says otherwise reads as + // broken just as surely as an empty one with no heading at all. + return _reachableOnly.Contains(steamId) && Standing(steamId) != null + ? "Throwable from here" + : null; + } + + private Vec3? Standing(ulong steamId) + { + IPlayer? player = _system.Find(steamId); + + return player == null ? null : PracticeSystem.Where(player)?.feet_position; } private string Tag(ulong steamId, int shown) diff --git a/apps/utility-sw/src/Services/PracticeLibrary.cs b/apps/utility-sw/src/Services/PracticeLibrary.cs index 754e31a3..8cc6ee3a 100644 --- a/apps/utility-sw/src/Services/PracticeLibrary.cs +++ b/apps/utility-sw/src/Services/PracticeLibrary.cs @@ -17,6 +17,25 @@ public class PracticeLibrary private readonly Dictionary> _lineups = new(); private string _map = ""; + // Everyone who asked for the same player's library while one request was + // already out, in the order they asked. Two requests in flight for one + // player landed in whatever order the panel answered, so the older one + // could overwrite what the newer one had already applied -- a pushed load + // that teleported someone and then had its markers redrawn a tick later. + private sealed class PendingFetch + { + public DateTime StartedAt; + + // The map it went out asking about. An answer for the map the server + // has left is no answer at all, so a request made after a changelevel + // cannot ride one made before it. + public string Map = ""; + + public List> Waiting { get; } = new(); + } + + private readonly Dictionary _fetching = new(); + public PracticeLibrary( ISwiftlyCore core, UtilityApiClient api, @@ -108,31 +127,107 @@ public void EnsureTrajectory(LineupRecord lineup, ulong steamId, Action? done = null) { + DateTime now = DateTime.UtcNow; string map = _map; + var fetch = new PendingFetch { StartedAt = now, Map = map }; - _ = Task.Run(async () => + if (_fetching.TryGetValue(steamId, out PendingFetch? existing)) { - List? lineups = await _api.Library(map, steamId); + bool expired = PracticeLibraryLoadUtility.IsFetchExpired(existing.StartedAt, now); - _core.Scheduler.NextTick(() => + if (!expired && existing.Map == map) { - if (lineups == null) + if (done != null) { - done?.Invoke(-1); - return; + existing.Waiting.Add(done); } - // The map can change while the request is in flight; dropping - // the answer beats showing inferno lineups on mirage. - if (map != _map) - { - done?.Invoke(-1); - return; - } + return; + } - _lineups[steamId] = lineups; - done?.Invoke(lineups.Count); - }); + if (expired) + { + // Presumed dead rather than merely slow. An in-flight marker + // that only an answer can clear is what pinned a player's + // library shut for the rest of the process. + _logger.LogWarning( + "library fetch for {steamId} never answered; asking again", + steamId + ); + } + + // Whoever was waiting on it rides the new request rather than + // being dropped along with the old one. + fetch.Waiting.AddRange(existing.Waiting); + } + + if (done != null) + { + fetch.Waiting.Add(done); + } + + _fetching[steamId] = fetch; + + _ = Task.Run(async () => + { + List? lineups = null; + + try + { + lineups = await _api.Library(map, steamId); + } + catch (Exception error) + { + // Nothing below this may be skipped: everything waiting on the + // answer is only ever released by the callback, so a throw that + // escaped here left the player's library pinned for good. + _logger.LogError(error, "unable to fetch the lineup library"); + } + + _core.Scheduler.NextTick(() => Landed(steamId, fetch, map, lineups)); }); } + + private void Landed( + ulong steamId, + PendingFetch fetch, + string map, + List? lineups + ) + { + // An answer to a request that was already given up on. The callers it + // was carrying moved to the one that replaced it, and its rows are the + // older of the two. + if (!_fetching.TryGetValue(steamId, out PendingFetch? current) || current != fetch) + { + return; + } + + _fetching.Remove(steamId); + + int count = -1; + + // The map can change while the request is in flight; dropping the + // answer beats showing inferno lineups on mirage. + if (lineups != null && map == _map) + { + _lineups[steamId] = lineups; + count = lineups.Count; + } + + foreach (Action waiting in fetch.Waiting) + { + try + { + waiting(count); + } + catch (Exception error) + { + // One caller throwing is not the rest of them missing their + // answer, and an exception let back out of here crosses into + // native SwiftlyS2 and takes the server down. + _logger.LogError(error, "a library refresh callback threw"); + } + } + } } diff --git a/apps/utility-sw/src/Services/UtilityApiClient.cs b/apps/utility-sw/src/Services/UtilityApiClient.cs index 87c51b1f..7d603ede 100644 --- a/apps/utility-sw/src/Services/UtilityApiClient.cs +++ b/apps/utility-sw/src/Services/UtilityApiClient.cs @@ -558,7 +558,24 @@ private sealed class SendOutcome { byte[]? response = await Send(method, path, body, outcome); - return response == null ? null : PracticeJson.Text(response); + if (response == null) + { + return null; + } + + try + { + return PracticeJson.Text(response); + } + catch (Exception error) + { + // A truncated or half-written gzip body throws out here rather than + // inside Send, where every other failure is already turned into "no + // answer". Callers read a null the same way; a throw escapes into + // whichever discarded task made the call and is never seen again. + _logger.LogError(error, "unreadable response body from {path}", path); + return null; + } } private async Task Send( diff --git a/apps/utility-sw/src/UtilityPractice.csproj b/apps/utility-sw/src/UtilityPractice.csproj index 424c540d..051f0267 100644 --- a/apps/utility-sw/src/UtilityPractice.csproj +++ b/apps/utility-sw/src/UtilityPractice.csproj @@ -52,6 +52,7 @@ + diff --git a/apps/utility-sw/src/UtilityPracticePlugin.cs b/apps/utility-sw/src/UtilityPracticePlugin.cs index 3d858444..73fc5984 100644 --- a/apps/utility-sw/src/UtilityPracticePlugin.cs +++ b/apps/utility-sw/src/UtilityPracticePlugin.cs @@ -56,6 +56,29 @@ public partial class UtilityPracticePlugin : BasePlugin // field: the plugin instance survives a map change, which is the only // reason this works at all. private PracticeMapChangePending? _pendingMapLoad; + + // What the drain knows about one player's library. The map is held rather + // than cleared on a change, so a fetch that lands on the wrong side of a + // changelevel does not count as done. + private sealed class LibraryLoad + { + public string? LoadedFor; + + public DateTime? FetchingSince; + + // A panel that cannot be reached stays unreachable for minutes, so the + // drain widens the gap between attempts rather than asking every second + // for the life of the server. + public int Failures; + public DateTime? RetryAt; + + // A refresh the panel pushed while a fetch was already out. That fetch + // predates whatever was edited on the website, so its answer is stale + // by definition and the ask is re-issued rather than dropped. + public bool Requeue; + } + + private readonly Dictionary _libraryLoads = new(); private EventDelegates.OnMapLoad? _mapLoadHandler; private EventDelegates.OnClientDisconnected? _disconnectHandler; private EventDelegates.OnPrecacheResource? _precacheHandler; @@ -218,6 +241,10 @@ public override void Load(bool hotReload) steamId => { _welcomed.Remove(steamId); + // Best effort only -- a player the engine has already + // invalidated never reaches this at all, which is why the + // drain prunes off the roster as well. + _libraryLoads.Remove(steamId); ForgetHud(steamId, @event.PlayerId); OnPlayerGone(steamId); } @@ -225,13 +252,13 @@ public override void Load(bool hotReload) }; Core.Event.OnClientDisconnected += _disconnectHandler; - // Refresh FETCHES; it does not draw. Somebody who joins and runs no - // command should still see every lineup on the map. + // Deliberately does not fetch the library: this fires before the map is + // known on a cold boot, which asked the panel for the lineups on map "" + // and then recorded that as the player's answer. The drain picks them up + // off the roster on the next second tick instead -- see DrainLibraryLoads. _authorizeHandler = @event => { _occupancyDirty = true; - - ForPlayer(@event.PlayerId, steamId => RefreshAndShow(steamId)); }; Core.Event.OnClientSteamAuthorize += _authorizeHandler; @@ -273,7 +300,43 @@ public override void Load(bool hotReload) ); } + // SwiftlyS2 hot reloads a plugin from a thread pool thread -- its file + // watcher schedules the reload through Task.Run -- and the game thread is + // never one of those. Every native call throws off the game thread, so the + // teardown below has to be handed back to it, and waited for: the moment + // this returns the reload puts a second instance in the world. public override void Unload() + { + if (!Thread.CurrentThread.IsThreadPoolThread) + { + TearDown(); + return; + } + + try + { + if (!Core.Scheduler.NextTickAsync(TearDown).Wait(TearDownWait)) + { + // The queued call holds the assembly load context open, so it + // still runs -- just late, next to whatever the new instance + // has already drawn. + _logger.LogWarning( + "teardown did not reach the game thread within {seconds}s", + TearDownWait.TotalSeconds + ); + } + } + catch (Exception exception) + { + _logger.LogWarning(exception, "teardown failed"); + } + } + + // A tick is milliseconds away. Only a server that has stopped running them + // waits this out, and nothing was coming down cleanly there anyway. + private static readonly TimeSpan TearDownWait = TimeSpan.FromSeconds(5); + + private void TearDown() { // Drawn entities are not the plugin's to leave behind: without this a // hot reload orphans every beam, label and model in the world, with no @@ -1390,6 +1453,7 @@ private void OnSecond() _solver.RefreshVisibility(); // A no-op once the map has answered; see MapCalloutsReporter.Report. _callouts.Report(_session.Map); + DrainLibraryLoads(); DrainPendingMapLoad(); } @@ -1692,35 +1756,201 @@ private void OnMapLoad(string mapName) // The panel is the only source of both the roster and the library, so a // refresh is one round trip followed by one per connected player. - private void RefreshAndShow(ulong steamId) + // + // Pushed is the panel or the player asking outright, which says the rows in + // hand are behind; the drain's own asks are not, and give way to whatever + // the player is in the middle of. + private void RefreshAndShow(ulong steamId, bool pushed) { - _library.Refresh( - steamId, - count => - { - if (count <= 0) - { - return; - } + // The map this fetch is answering for. Compared again when it lands, + // because a changelevel in between makes the answer worthless. + string map = _library.Map; + + if (string.IsNullOrEmpty(map)) + { + return; + } + + LibraryLoad load = LibraryLoadFor(steamId); + DateTime now = DateTime.UtcNow; + + // At most one in flight per player. A pushed refresh cannot ride the + // one already out -- that request predates whatever the panel is + // telling us about -- so it is re-asked the moment this one lands. + if ( + load.FetchingSince != null + && !PracticeLibraryLoadUtility.IsFetchExpired(load.FetchingSince.Value, now) + ) + { + load.Requeue |= pushed; + return; + } + + load.FetchingSince = now; + + _library.Refresh(steamId, count => LibraryLanded(steamId, map, count, pushed)); + } + + private void LibraryLanded(ulong steamId, string map, int count, bool pushed) + { + LibraryLoad load = LibraryLoadFor(steamId); + + load.FetchingSince = null; + + bool requeue = load.Requeue; + load.Requeue = false; + + // Left while the panel was answering. Marking them done would outlive + // them: the entry is keyed by steam id, and a rejoin would be skipped + // by the drain. + if (_system.Find(steamId) == null) + { + _libraryLoads.Remove(steamId); + return; + } - IReadOnlyList library = _library.For(steamId); + // Below zero is "could not reach the panel", or the map moved under the + // request. Deliberately not marked, so the drain asks again -- but on a + // widening delay: a panel that is down stays down, and one request per + // player per second for the life of the server is a storm, not a retry. + // An empty library IS an answer. + if (count < 0) + { + load.Failures++; + load.RetryAt = DateTime.UtcNow + PracticeLibraryLoadUtility.RetryDelay(load.Failures); + return; + } - _replay.ShowLibrary(library); + load.Failures = 0; + load.RetryAt = null; + load.LoadedFor = map; - // .next and .prev walk state.Results, and a refresh never filled - // it -- so every lineup on the map was drawn and none of them - // could be stepped through until the player ran a search. If - // they can SEE them, they can walk them. Any earlier search is - // discarded on purpose: this only runs on join, map change and - // an explicit refresh, and a search from before any of those is - // describing a map state that no longer exists. - PracticeState state = _system.StateFor(steamId); + if (count > 0) + { + IReadOnlyList library = _library.For(steamId); + + ShowLibraryFor(steamId, library); + + // .next and .prev walk state.Results, and a refresh never filled it + // -- so every lineup on the map was drawn and none of them could be + // stepped through until the player ran a search. If they can SEE + // them, they can walk them. + // + // Only when the panel pushed, or when there is nothing to lose: the + // drain retries at a moment nobody chose, and replacing the results + // of a .load or a target picked off the minimap turns somebody's + // walk into the whole map halfway through it. + PracticeState state = _system.StateFor(steamId); + if (pushed || state.Results.Count == 0) + { state.Results.Clear(); state.Results.AddRange(library); state.Index = -1; + + if (!pushed) + { + Tell( + steamId, + $" {ChatColors.Green}{count} lineup(s) {ChatColors.Grey}on " + + $"{ChatColors.Default}{map} {ChatColors.Grey}-- " + + $"{ChatColors.Default}.next{ChatColors.Grey} to walk them" + ); + } } - ); + } + + if (requeue) + { + RefreshAndShow(steamId, pushed: true); + } + } + + // Markers are one shared set of entities for the whole server (see + // PracticeReplay.ShowLibrary) while a library is filtered per player by the + // panel, so drawing one on somebody's behalf only holds while there is + // nobody else it could be shown to. With company it drew whoever's fetch + // landed last, which put one player's private lineups in front of everyone. + // Their own .load, .next and .menu still draw, because those were asked for. + private void ShowLibraryFor(ulong steamId, IReadOnlyList library) + { + List connected = _system.ConnectedSteamIds(); + + if (connected.Count != 1 || connected[0] != steamId) + { + return; + } + + _replay.ShowLibrary(library); + } + + private LibraryLoad LibraryLoadFor(ulong steamId) + { + if (!_libraryLoads.TryGetValue(steamId, out LibraryLoad? load)) + { + load = new LibraryLoad(); + _libraryLoads[steamId] = load; + } + + return load; + } + + /** + * Everybody in the server ends up holding the library for the map they are + * standing in, without anybody having typed .load. + * + * Driven off the second tick for the same reason the pending map load is: + * a changelevel puts every client through its own reconnect, and a client + * event that fires on the wrong side of it either finds no players yet or + * fetches the map the server is leaving. Both used to end with a player + * arriving on a new map to an empty library and no markers. Reconciling + * costs one lookup per player per second and does not care which hook fired. + * + * What it costs the PANEL is decided by PracticeLibraryLoadUtility: a hit + * once per map per player, and after a failure a widening wait rather than + * another request every second. + */ + private void DrainLibraryLoads() + { + string map = _library.Map; + + if (string.IsNullOrEmpty(map)) + { + return; + } + + List connected = _system.ConnectedSteamIds(); + + // Entries outlive the disconnect hook in both directions -- the leaving + // player is still on the roster when it fires, and one the engine has + // already invalidated never resolves there at all -- so the roster this + // loop is holding anyway is the only thing that cannot go stale. + foreach (ulong gone in _libraryLoads.Keys.Where(id => !connected.Contains(id)).ToList()) + { + _libraryLoads.Remove(gone); + } + + DateTime now = DateTime.UtcNow; + + foreach (ulong steamId in connected) + { + _libraryLoads.TryGetValue(steamId, out LibraryLoad? load); + + if ( + !PracticeLibraryLoadUtility.ShouldFetch( + map, + load?.LoadedFor, + load?.FetchingSince, + load?.RetryAt, + now + ) + ) + { + continue; + } + + RefreshAndShow(steamId, pushed: false); + } } private void RefreshEverything() @@ -1735,7 +1965,7 @@ private void RefreshEverything() { if (player != null && player.IsValid && !player.IsFakeClient) { - RefreshAndShow(player.SteamID); + RefreshAndShow(player.SteamID, pushed: true); } } } diff --git a/apps/utility-sw/test/FiveStack.Tests.csproj b/apps/utility-sw/test/FiveStack.Tests.csproj index e06a3e9d..a91ad3cc 100644 --- a/apps/utility-sw/test/FiveStack.Tests.csproj +++ b/apps/utility-sw/test/FiveStack.Tests.csproj @@ -42,6 +42,7 @@ + diff --git a/apps/utility-sw/test/PracticeLibraryLoadUtilityTests.cs b/apps/utility-sw/test/PracticeLibraryLoadUtilityTests.cs new file mode 100644 index 00000000..90b3c0e5 --- /dev/null +++ b/apps/utility-sw/test/PracticeLibraryLoadUtilityTests.cs @@ -0,0 +1,142 @@ +using FiveStack.Utilities; +using Xunit; + +// The drain runs once a second for every player on the server, against a panel +// that can be slow, down or answering about a map the server has already left. +// Every one of those turns into a request nobody asked for if the rules here +// are wrong, so they are pinned rather than left in the plugin. +public class PracticeLibraryLoadUtilityTests +{ + private static readonly DateTime Now = new DateTime(2026, 8, 29, 12, 0, 0, DateTimeKind.Utc); + + [Fact] + public void FetchesForAPlayerWhoHasNothing() + { + Assert.True(PracticeLibraryLoadUtility.ShouldFetch("de_mirage", null, null, null, Now)); + } + + [Fact] + public void LeavesAPlayerWhoAlreadyHasThisMapAlone() + { + Assert.False( + PracticeLibraryLoadUtility.ShouldFetch("de_mirage", "de_mirage", null, null, Now) + ); + } + + // The whole reason the load is keyed by map: an answer that arrived for the + // map the server was leaving is not an answer for the one it is on. + [Fact] + public void FetchesAgainAfterAMapChange() + { + Assert.True( + PracticeLibraryLoadUtility.ShouldFetch("de_nuke", "de_mirage", null, null, Now) + ); + } + + // A cold boot has no map until OnMapLoad. Asking for the library on "" is a + // wasted round trip that then gets recorded as this player's answer. + [Fact] + public void WaitsForTheMapToBeKnown() + { + Assert.False(PracticeLibraryLoadUtility.ShouldFetch("", null, null, null, Now)); + } + + [Fact] + public void DoesNotStackASecondFetchOnAnOutstandingOne() + { + Assert.False( + PracticeLibraryLoadUtility.ShouldFetch( + "de_mirage", + null, + Now.AddSeconds(-5), + null, + Now + ) + ); + } + + // The failure this exists for: an answer that never lands leaves the + // in-flight marker set, and a marker only ever cleared by an answer stops + // that player's library loading for the rest of the process. + [Fact] + public void GivesUpOnAFetchThatNeverAnswered() + { + DateTime issued = Now.AddSeconds(-PracticeLibraryLoadUtility.FetchTimeoutSeconds - 1); + + Assert.True(PracticeLibraryLoadUtility.IsFetchExpired(issued, Now)); + Assert.True( + PracticeLibraryLoadUtility.ShouldFetch("de_mirage", null, issued, null, Now) + ); + } + + [Fact] + public void ARequestInsideTheTimeoutIsStillAlive() + { + DateTime issued = Now.AddSeconds(-PracticeLibraryLoadUtility.FetchTimeoutSeconds + 1); + + Assert.False(PracticeLibraryLoadUtility.IsFetchExpired(issued, Now)); + } + + [Fact] + public void HoldsOffUntilTheBackoffHasRunOut() + { + Assert.False( + PracticeLibraryLoadUtility.ShouldFetch( + "de_mirage", + null, + null, + Now.AddSeconds(3), + Now + ) + ); + + Assert.True( + PracticeLibraryLoadUtility.ShouldFetch( + "de_mirage", + null, + null, + Now.AddSeconds(-1), + Now + ) + ); + } + + // An unreachable panel used to cost one request per player per second for + // the life of the server. Ten people on a practice box whose panel is down + // is a request storm, and it never stopped on its own. + [Fact] + public void BacksOffFurtherOnEveryFailure() + { + Assert.Equal(TimeSpan.FromSeconds(2), PracticeLibraryLoadUtility.RetryDelay(1)); + Assert.Equal(TimeSpan.FromSeconds(4), PracticeLibraryLoadUtility.RetryDelay(2)); + Assert.Equal(TimeSpan.FromSeconds(8), PracticeLibraryLoadUtility.RetryDelay(3)); + Assert.Equal(TimeSpan.FromSeconds(16), PracticeLibraryLoadUtility.RetryDelay(4)); + } + + // Capped rather than unbounded: a panel that comes back after an hour is + // picked up within the minute, not the next day. + [Fact] + public void NeverWaitsLongerThanAMinute() + { + Assert.Equal( + TimeSpan.FromSeconds(PracticeLibraryLoadUtility.MaxRetrySeconds), + PracticeLibraryLoadUtility.RetryDelay(10) + ); + + Assert.Equal( + TimeSpan.FromSeconds(PracticeLibraryLoadUtility.MaxRetrySeconds), + PracticeLibraryLoadUtility.RetryDelay(int.MaxValue) + ); + } + + // A server left running against a dead panel reaches a failure count that + // overflows a naive shift, and a negative delay retries instantly forever. + [Fact] + public void NeverReturnsANegativeDelay() + { + foreach (int failures in new[] { 0, 1, 31, 32, 33, 64, int.MaxValue }) + { + Assert.True(PracticeLibraryLoadUtility.RetryDelay(failures) > TimeSpan.Zero); + } + } +} diff --git a/shared/dotnet/FiveStack.Utilities/PracticeLibraryLoadUtility.cs b/shared/dotnet/FiveStack.Utilities/PracticeLibraryLoadUtility.cs new file mode 100644 index 00000000..b5f231f2 --- /dev/null +++ b/shared/dotnet/FiveStack.Utilities/PracticeLibraryLoadUtility.cs @@ -0,0 +1,77 @@ +namespace FiveStack.Utilities; + +// When a player's saved lineups are worth (re)fetching, kept out of the plugin +// for the same reason the map change is: the drain runs off the second tick +// against a panel that can be slow, wrong or absent, and every one of those +// cases is a rule rather than a game object. +public static class PracticeLibraryLoadUtility +{ + // A fetch is one HTTP call with a ten second timeout, so anything still + // outstanding after this is not coming back -- the answer was dropped + // somewhere between the request and the tick meant to apply it. Presuming + // it dead is the only thing that unpins the player: an in-flight marker + // that is only ever cleared by an answer stops the library loading again + // for the rest of the process. + public const int FetchTimeoutSeconds = 30; + + // A panel that is down stays down for minutes, not milliseconds. Doubling + // from two seconds keeps a blip invisible, and the cap turns an outage into + // one request per player per minute instead of sixty. + public const int MinRetrySeconds = 2; + public const int MaxRetrySeconds = 60; + + public static TimeSpan RetryDelay(int failures) + { + if (failures <= 1) + { + return TimeSpan.FromSeconds(MinRetrySeconds); + } + + // Clamped before the shift as well as after it: a server left running + // against a dead panel reaches a failure count that overflows one. + int doublings = Math.Min(failures - 1, 8); + int seconds = Math.Min(MinRetrySeconds << doublings, MaxRetrySeconds); + + return TimeSpan.FromSeconds(seconds); + } + + public static bool IsFetchExpired(DateTime startedAt, DateTime now) + { + return now - startedAt >= TimeSpan.FromSeconds(FetchTimeoutSeconds); + } + + /// + /// Whether the drain should ask the panel for this player's library. + /// + /// The map the server is on; empty before one is known. + /// The map the library in hand is for, if any. + /// When the outstanding fetch was issued, if any. + /// When the next attempt is allowed after a failure. + public static bool ShouldFetch( + string map, + string? loadedFor, + DateTime? fetchingSince, + DateTime? retryAt, + DateTime now + ) + { + // A cold boot has no map until OnMapLoad, and a library for "" is a + // wasted round trip that would then be recorded as loaded. + if (string.IsNullOrEmpty(map)) + { + return false; + } + + if (fetchingSince != null && !IsFetchExpired(fetchingSince.Value, now)) + { + return false; + } + + if (string.Equals(loadedFor, map, StringComparison.Ordinal)) + { + return false; + } + + return retryAt == null || now >= retryAt.Value; + } +}