Repository navigation
fix(bots): keep a bot script's Lua state across think calls, and bound it - #571
Merged
Merged
Conversation
asobi_bot called think/2 and threw away the Luerl state it got back, so every tick re-entered the script exactly as it was loaded. A bot script could not remember anything between calls: a per-bot cooldown table reset every 100 ms and the guard built on it fired on every tick, which is what a forum poster hit trying to make a bot wait four seconds between commands. The guide told them to use that exact pattern. Thread the state through send_input/1 instead. Persisting it means a bot now pays the per-tick copy a match bridge pays, so run asobi_lua_loader:collect_state/1 before each call on the same adaptive schedule. The bot holds no Luerl reference across ticks - game_state is the Erlang term the match broadcast, re-encoded every tick - so the collector gets no anchor. A think that returns no values keeps its state; only the input falls back to the default AI. A think that errors, times out or overruns its heap keeps the pre-call state, as every other bounded caller does. Guide gains a "Timing and cooldowns" section covering both clocks: count your own calls, or count a tick counter the match broadcasts, with the note that think runs on a fixed 100 ms loop independent of tick_rate. ADR 0005 gains kind => bot on [asobi, lua, state].
Follow-up to b1cef15, applying the gate's findings. Persisting the Luerl state removed the only bound a bot's Lua memory had. luerl:gc/1 reclaims garbage, not what a script keeps live, and its mark phase is quadratic in the live set, so a hoarding script drives the adaptive collector past ?GC_ABANDON_US and it switches itself off for that state: measured 139 MB per bot after 400 ticks and a mean tick of 104 ms against a 100 ms loop. collect_state/1 now returns the size it already measured, and a bot past max_bot_state_words (10 MB, overridable under {asobi, [...]}) reloads its script. A bot is the one Lua bridge where that is affordable - nothing a player can see lives in its state. decode_result/2 gains a try/catch: luerl:decode/2 raises on a table that references itself, and an uncaught raise kills a `temporary` child, so the match lost the bot for the rest of its life. The gen_server also gains format_status/1 - a crash report formatting a multi-MB state measured 10 MB of log and 1.4 s. The think-error log bounds the reason it echoes, since a script chooses what it raises with and can raise with megabytes. Its throttle moves from persistent_term into the bot's own state: bot ids carry a random discriminator and are never reused, so the old key was a table that only grew, and every write scanned the literal area across all processes. An input reaching the match script drops funs, pids, ports and refs. Nested tables are kept - a bot may send the shaped input a player can. send_input/1 splits into run_think/4 and collect_lua_state/1, and the bridge metadata is stamped once in init/1 the way asobi_lua_match and asobi_lua_world do it, so collect_lua_state/1 is maps:without/2 on the anchor rather than a hand-built map. Tests cover the four paths the gate found untested, each verified to fail when its own fix is neutralised: a bare `return` (zero values, distinct from `return {}`) keeping state, the ceiling reload, a self-referencing result, and a returned function. Docs: the bots guide gains "What a bot may keep" and now says a bare `return` runs the default AI rather than declining, and that a failed call is rolled back whole so a deterministic failure repeats forever. ADR 0005 no longer claims eight bots are eight series - they collapse on script/kind, and bot_id must never be a label. asobi_telemetry and guides/observability.md catch up with the enum. The trust model notes that the state copy sits outside the callback budget. Behaviour changes for anyone already running bots: a bot script's globals now persist (b1cef15), a script holding more than 10 MB is reloaded, and the think-error log carries the message under `event` rather than `msg`.
🟡 Code Coverage — 76.9%9423 of 12258 lines covered. 🟡 ELP Lint — 192 warnings212 diagnostics found. See job logs for details. ✅ ELP eqWAlizeNo diagnostics. |
Two CI failures, both in the module this branch touches. Dialyzer: run_think/4's catch-all clause is unreachable now that the three shapes asobi_lua_loader:call/4 can return are all matched. eqWAlize: handle_presence_message/2 was spec'd on asobi_presence:message/0 while its only caller passes what arrived in the mailbox, which is term(). That error predates this branch, but the CI gate is scoped to modules a PR touches, so it surfaces here. Spec the consumer as dynamic() - a mailbox is a boundary and the clause heads are what decide the shape - and leave the contract enforced where it always was, on the producing asobi_presence:send/2 call.
Taure
added
the
gates-passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
asobi_botcalled the operator's Luathink(bot_id, state)and threw away the Luerl state it got back, so every tick re-entered the script exactly as it was loaded. A bot script could not remember anything between calls.That made
guides/lua-bots.mdwrong where it told authors to key per-bot state offbot_idin a module-level table, and it is what a forum poster hit trying to make a bot wait four seconds between commands. Their client-side guard fired every tick because the value it compared against had already reset. Working around it server-side was the only thing that could have worked.Reproduced before fixing, and the regression tests fail on
mainwith the exact symptom:What changed
The state is threaded across ticks (
b1cef15), so a bot script's globals persist the way a match script's do. On failure the pre-call state is carried forward, so a bot that fails every tick cannot grow the state it is failing on; athinkthat returns no values keeps its state and only the input falls back.And then bounded (
2971a17). Persisting removed the only limit a bot's Lua memory had:luerl:gc/1reclaims garbage, not what a script keeps live, and its mark phase is quadratic in the live set, so a hoarding script drives the adaptive collector past its abandon threshold and it switches off for that state. Measured 139 MB per bot after 400 ticks, with a mean tick of 104 ms against a 100 ms loop.collect_state/1now returns the size it was already measuring, and a bot pastmax_bot_state_words(10 MB, configurable) reloads its script. A bot is the one Lua bridge where that is affordable: nothing a player can see lives in its state.Also hardened on the same path:
decode_result/2gains a try/catch.luerl:decode/2raises on a self-referencing table, and an uncaught raise kills atemporarychild, so the match lost the bot for the rest of its life.format_status/1, so a crash report does not format a multi-MB state (measured 10 MB of log, 1.4 s).persistent_term, whose keys were per bot id and therefore never reused.Docs
guides/lua-bots.mdgains Timing and cooldowns (count your ownthinkcalls, or count a tick counter the match broadcasts) and What a bot may keep. It also now says thatthinkruns on a fixed 100 ms loop independent oftick_rate, that a barereturnruns the default AI rather than declining, and that a failed call is rolled back whole so a deterministic failure repeats forever.ADR 0005,
asobi_telemetryandguides/observability.mdpick upkind => bot, withbot_idexplicitly not a label. The trust model notes that the state copy sits outside the callback budget.Review
Ran the full gate. The architecture guardian accepted the design and independently reproduced the bug; the security reviewer raised the memory ceiling as a merge blocker, which is what
2971a17addresses; the code reviewer found the{ok, [], _}clause untested.Six regression tests, each verified to fail when its own fix is neutralised.
fmt/xref/dialyzer/eqwalizeclean, fulleunit2406 passing,asobi_lua_SUITE8/8. DB-backed CT suites were not run locally: Docker is not reachable from this machine.Behaviour changes for anyone already running bots
thinkcalls.eventrather thanmsg. The string is unchanged, so a grep still matches.