Skip to content

sharing 1/7: Transfer library (enums, comms, serialization, test infra) - #8125

Closed
keithharvey wants to merge 15 commits into
fmt-llmfrom
sharing/01-foundations
Closed

sharing 1/7: Transfer library (enums, comms, serialization, test infra)#8125
keithharvey wants to merge 15 commits into
fmt-llmfrom
sharing/01-foundations

Conversation

@keithharvey

@keithharvey keithharvey commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

📚 The sharing stack — review bottom-up

Note

Part of native stack #8411 (feature tracking: #8412). Merges bottom-up through the GitHub stacked-PR UI once the type-migration stack (#8398) is in.

Each PR is file-partitioned: every file appears in exactly one PR in its final sharing_tab form, so each PR's diff is byte-identical to that branch. Regenerated deterministically by just bar::sharing-split.

Summary (LLM-generated, claude-opus-4-6)

Introduces the foundational type vocabulary and shared library code for the team transfer system: TransferEnums (with PolicyType, ResourceCommunicationCase, UnitCommunicationCase, UnitType), the PolicyResult / PolicyContext / ResourceData type definitions, and sharing_mode_enums for modoption keys and mode values. Adds comms modules (resource_transfer_comms, unit_transfer_comms, take_comms, tech_blocking_comms) that map a PolicyResult into communication cases, tooltip text, and chat messages. Provides wire-format serialization (lua_rules_msg, team_transfer_serialization_helpers), a unit-def classifier (unit_sharing_categories), a PolicyEvents.NotifyIfChanged change-tracker, and supporting test infrastructure (builder extensions, spec helpers, busted specs for PolicyEvents and unit categories).


My [no LLM editor] Description

Intro

So I'm going to review each line in these PRs, then write in my own words this description/architectural overview. I got help with converting the mermaid diagram from my markdown equivalent pseudo code and had it insert links, but otherwise it's untouched by LLMs -- all me baby. Hopefully this can explain what is going on and put a more human voice on this design. Going to try to keep these descriptions short and to the point, with the exception of this PR because I need to provide a little context to get people going on this train of thought.

The core conceit here is that for a subset of existing game behavior, we own execution end to end. This is me applying my bag of tricks from doing this type of refactor to this type of system countless times. It is largely a UI problem to me, and my usual way to approach that category of problem borrows heavilly from reactive programming. This PR uses the combination of a service layer in the form of the unit_transfer_controller and resource_transfer_controller and then below that a policy pattern or game behavior in order to encapsulate state and categorize commonality between behaviors in our types at various points in our functional execution layer.

PolicyType

The first real thing to understand is the PolicyType enum, which represents every type of behavior our modules can express:

  • metal_transfer
  • energy_transfer
  • unit_transfer

Note that this enum could easilly map ALL behavioral categories as part of a more opinionated framework. As we expanded this pattern to other behaviors, this list would grow with the number of behavioral subsystems we refactored into. Therefore, it is important to remember that a lot of the boilerplate orchestrator functions here are truly framework code, even if they're probably not in their final form or directory. They're still a pure function that represents the same exact parameters they will ultimately have under a more opinionated framework. And they are subsequently highly portable.

How does it fit together?

So we have PolicyType, but what are the other pieces of a unified game side execution in these categories?

Basically, it's:

flowchart TD
    Engine[Engine]

    subgraph Synced
        subgraph SL["Internal Service Layer"]
            direction TB
            Controller["behavior_controller<br/>(game_unit_transfer_controller, …)<br/>executes commands within<br/>bounds set by PolicyResult"]
            Context["Context<br/>(cached)"]
            Policy[Policy]
            Result[PolicyResult]
            Controller --> Context --> Policy --> Result
            Result -.->|bounds execution| Controller
        end
        Gadgets["External gadgets"]
    end

    subgraph Unsynced
        UI[UI]
    end

    Command["«command»<br/>GG.* action request<br/>(independent data type)"]
    classDef iface fill:none,stroke:#888,stroke-width:2px,stroke-dasharray:6 4;
    class Command iface

    Engine --> Controller
    Result -->|published cache| Gadgets
    Result -->|published cache| UI
    Gadgets -.->|send| Command
    UI -.->|send| Command
    Command -.->|request| Controller
Loading

Let's talk through each piece of that architecture in order.

PolicyResult

These are the central goal: establish a "view model" simplifying the game behavior matrix for downstream consumers. This construct is that view model. We're skipping a few steps here but don't worry we'll come back to those other execution steps in a second.

Let's start with an example, the UnitPolicyResult:

See UnitPolicyResult, which extends PolicyResult.

This is our authoritative matrix of game behaviors as it pertains to the PolicyType=unit_transfer and the type itself existing is inherently simplifying. It is portable across layers in a way that unifies code that deals with that behavior category. Every downstream system just has to consume this type in order to understand every permutation of behavior possible. Each key is orthogonal behavior by design.

Seeing PolicyType and a functional file scoped to a particular one is self-descriptive. We get a way to speak about game behavior at runtime with type enforcement -- in a way that current patterns have major downsides described in the Architecture document here.

Engine -> Behavior Controller

This part is pretty easy because it's just straight up service layer encapsulating state from an external API. It is the master of its own internal state and all downstream consumers talk to this layer, so it can be confident in its factoring that it is just ensuring its own internal state gets updated correctly and it responds to all engine requests faithful to the wishes expressed by that internal state engine.

Context (cached inputs)

So PolicyResult can only exist with boilerplate that enables them to have their inputs disconnected from the engine. You need a type to express your explicit inputs from the engine to allow hot-swappability and testability for your specific engine API surface. And you also need to build it performantly -- in Lua 5.1. That's where ContextFactory comes in. It's only job is to build structured, memoized state that can be cached from the engine, that the policies then use to initialize a per-team cache to drive the UI/everything else.

This is some of that "framework" code we talked about earlier. It's common to all types of game behavior and allows us to white list engine state we care about, in the shape we care about it. It is extensible from downstream consumers of a given behavioral service layer (ie game_unit_transfer_controller).

Here is an example from the same beahvioral vertical we have been looking at (unit_transfer) of a PolicyContext:

See PolicyContext.

This gives a future developer an explicit understanding of the input data we need and cache for a given behavior expression.

Policies

Policies produce PolicyResult and because we have a clear input and output type, are extremely unit testable.

Here is the

Notice how they are stateless, and do complicated things. But the result is extremely simple for consumers. We do as much work as we can here because it's cached. This reduces complexity for devs that just want to bring their own policy, or make simple modifications to ours. We bound the runtime cost by providing and caching the policies. We can be as expressive as we want on top of that, whether that's this implementation or something that actually builds a clean AST and is more opinionated internally.

Commands (Actions)

This same pattern is used throughout the service layer. We establish clear inputs and outputs in the form of types for a given behavior, and then ensure our execution code conforms to the boundaries established by the PolicyResult.

See team_transfer/unit_transfer_synced.lua

UI

It and supporting functions in the widget layer are all simply fluent in PolicyResult. Very simple, and very easy to rip out functional slices of behavior from things like gui_chat or gui_advplayerslist because you are just coding to the type already, anything that talks about PolicyResult is easy to rip out because it's inherently reactive and scoped.

1/7 Specific PR Analysis

So that's the meat of it. This PR specifically attempts to lay the ground work by introducing

  • all of the types -- including PolicyType and the PolicyResult types pulled into this system, the various inputs and outputs, internal and external, for the synced-layer team_transfer APIs

  • a textbook and generic NotifyIfChanged provides change tracking for a given PolicyResult

  • Comms files provide my take on a classifier using PolicyResult to map complicated, fractured requirements into simple functional code that is easy to reason about.

    • Unit Transfer Comms - team_transfer/unit_transfer_comms.lua
      • DecideCommunicationCase
      • TooltipText - one function that generates every tooltip as it pertains to unit_transfer (e.g. when you hover over the button to do that with a given selection). It provides tooltip information relevant to a given unit selection and PolicyResult, which allows it to be VERY specific for players that might be confused about a litany of configurations the game might be in during any given moment, without letting that complexity leak into gui_advplayerlist or gui_chat.

    I am SUPER proud of these implementations because they were the most difficult part of this refactor and "doing it right" if you have independently configurable mod options for a given behavior. So they got distilled down to a fine wine reduction of the problem space and I think represent a good demonstration of how much complexity you can disappear with this work.

  • things like the unit_sharing_categories.lua is another classifier and used by features like "stun delay category" to target a specific unit "group".

  • you can tell I really like enums

  • this PR does contain 2 bug fixes:

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Integration Test Results

14 tests  ±0   6 ✅ ±0   3s ⏱️ ±0s
 1 suites ±0   8 💤 ±0 
 1 files   ±0   0 ❌ ±0 

Results for commit fb30fe1. ± Comparison against base commit 570ed55.

♻️ This comment has been updated with latest results.

@@ -13,9 +13,9 @@ function gadget:GetInfo()
}
end

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code hardcoded string.sub(msg, 4) to skip a 3-char "msg" header, which left a stray leading colon on every routed message ("msg:ui" -> ":ui"). The fix makes the header "msg:" explicit and strips it via PACKET_HEADER_LENGTH killing the magic number and the leading colon in the process. This quirk exists on master but is coupled to changes I made in gui_chat, so I left it here and didn't strip this one out.

@keithharvey
keithharvey force-pushed the sharing/01-foundations branch from bfed399 to 3d0c4b3 Compare June 30, 2026 07:05
local function unescapeDoublePercent(str)
return str:gsub(ltDummy, "%%<"):gsub(bracketDummy, "%%{")
end

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also lives #7980

@keithharvey
keithharvey force-pushed the sharing/01-foundations branch 4 times, most recently from 8fbffb2 to de59010 Compare July 4, 2026 06:37
@keithharvey
keithharvey force-pushed the sharing/01-foundations branch 2 times, most recently from 5bb31c9 to 46406c0 Compare July 10, 2026 09:40
@keithharvey
keithharvey changed the base branch from sharing_tab_mergeable to fmt-llm July 10, 2026 09:41
@keithharvey
keithharvey force-pushed the sharing/01-foundations branch 3 times, most recently from 04b1c7a to c331a03 Compare July 15, 2026 07:33
@keithharvey
keithharvey force-pushed the sharing/01-foundations branch from c331a03 to f67a0d2 Compare July 15, 2026 20:05
@keithharvey
keithharvey force-pushed the sharing/01-foundations branch from e2a7cfb to 22dc1bd Compare July 17, 2026 08:00
keithharvey and others added 14 commits July 22, 2026 13:22
…ring

Renames spec/builders/{spring->engine}_{,un}synced_builder.lua + their
builder_specs, Builders.Spring->Builders.EngineSynced,
Builders.SpringUnsynced->Builders.EngineUnsynced, @Class names, and all
call sites. The renamed builders own every Engine-related test edit so no
two prereqs touch one file:
  - engine_synced_builder.lua: gamedata/system.lua defs mock aliases
    Engine.{Shared,Synced,Unsynced} + BAR to the _G.Spring/_G.BAR mocks
  - engine_unsynced_builder.lua: widget sandbox gets env.Engine; capture
    spies install on Engine.Shared.*

Prefix branch -> lands in fmt once, every leaf+mig inherits it.

Recovered from origin/mig-spring-split@4e25bfe82a (branch lost in a rename);
files transplanted verbatim (formatting is normalized by run_fmt).
Add Utilities, I18N, Debug, Lava, and GetModOptionsCopy to the
System tables in luaui/system.lua and luarules/system.lua so that
widgets and gadgets can access them after detach-bar-modules moves
them off the Spring table.

Also create .emmyrc.json (the EmmyLua analyzer config) with the
detached modules in the globals list, plus type stubs for LSP/CLI
support. The .emmyrc.json content matches what
vscode-recommended-extensions ships, with 5 extra globals
(Utilities/Debug/Lava/GetModOptionsCopy/I18N) that only become
real top-level identifiers after detach-bar-modules runs. When
vscode-recommended-extensions has already merged, -Xtheirs in the
cherry-pick keeps this version (the superset).

# Conflicts:
#	.emmyrc.json
Restructures the 20 files under luaui/Tests/, luaui/TestsExamples/,
plus the headless-only common/testing/infologtest.lua, from bare-
global hook declarations to a return-table shape. Updates the
dbg_test_runner widget to read test hooks from the returned table.

Motivation: the pre-existing shape required the test files to run
under setfenv(chunk, testEnvironment) and define `function test()`,
`function setup()`, etc. as bare module-level globals that setfenv
redirected into the environment. That works at runtime but emmylua
can't model the sandboxing — it sees 20+ files declaring project-
wide globals like `test`, `setup`, `skip`, `cleanup`. To keep
emmylua happy, .emmyrc.json had to blacklist both test directories
under workspace.ignoreDir — a kludge on clearly-ours code. Lives on
its own leaf so the convention change can be discussed in isolation.

Minimal shape change per file — just prepend `local` to each top-
level `function` declaration, and append a final `return { ... }`
block listing whichever lifecycle hooks (skip/setup/test/cleanup)
that file actually defines. Original indentation and formatting
preserved (no stylua reformatting noise — the fmt transform runs
after this one in the mig pipeline).

Runner patch — luaui/Widgets/dbg_test_runner.lua, loadTestFromFile:
  - capture the return value of pcall(chunk)
  - require it to be a table
  - merge its keys into testEnvironment so runTestInternal still
    reads bare `skip`/`setup`/`test`/`cleanup` under setfenv
Vendored LuaCATS annotations for busted/luassert to provide
IntelliSense for the unit-test surface. Lives on its own leaf so the
discussion around 'vendoring LuaCATS types' can happen in isolation —
prior pushback on the same direction in an earlier unit-testing PR
makes this the right place to litigate it rather than burying it in a
broader env commit.

Why vendored instead of declared as a Lux dep: Lux does not yet
support pulling LuaCATS annotations from library deps, and quick
attempts to wire this up in Lux failed. Upstream tracking issue:
lumen-oss/lux#953 — once that lands, these
directories should be deleted in favor of declaring busted as a
normal Lux dev-dep.

Sources (pinned SHAs):
  - types/busted/   https://github.com/LuaCATS/busted
                    @ 5ed85d0e016a5eb5eca097aa52905eedf1b180f1
  - types/luassert/ https://github.com/LuaCATS/luassert
                    @ d3528bb679302cbfdedefabb37064515ab95f7b9

See types/busted/provenance.md and types/luassert/provenance.md for
per-directory upstream refs + license status.
…ixes

Human-curated environment that, together with the LLM type-triage pass
(fmt-llm), drives emmylua_check to zero on the migrated tree. Per-change
rationale lives in PR #7447 review comments.

- .emmyrc.json globals + diagnostics; types/* stubs; busted mock; CI gate
- forward-decl / assertEqual declarations; reverted orphaned kikito loader
- rationale-comment strip; types/IntegrationTests rename
- deterministic pins for type-triage leftovers the LLM mishandles on big
  files (multi_attack opts, HighlightUnit forward-decl, ripairs suppress,
  gui_pip gameFrame use-before-declare)
- json.lua forward-decl tidy (relocate null, drop dead decode_scan*)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IsDevModeCached (upstream #6918) references `utilities` inside its own
table constructor, where the local is not yet in scope — Lua resolves
those reads as GLOBALS, so the first real call would index nil. Dormant
today only because nothing calls it. Forward-declare the local so the
closure captures it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0.22.0's analyzer resolves a local inside its own table constructor, so
constructor-self-reference globals — a real dormant-crash class (see the
springFunctions.lua fix) — passed CI silently. 0.24.0 catches them; the
whole workspace surfaces exactly the four occurrences of that one bug,
fixed in the previous commit, so the stricter gate lands green. Also
tracks the upstream release asset rename (arm64 -> aarch64).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gui_flowui.lua: Draw.Button uses `opaque` — Draw.Element's 17th
parameter, which Button never had; the global read was always nil.
Pinned false (behavior-identical) until upstream decides whether Button
should expose an opaque mode.

snd_notifications.lua: `customNotifications` is persisted by
GetConfigData but never declared or assigned anywhere. Declared nil so
the round-trip is explicit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated by parallel claude-sonnet-4-6 workers dispatched by
scripts/codemod/llm-type-triage.sh, applying fixes per SKILL.md categories.
Single pass, no iteration — categories that don't shrink the count
are a signal that SKILL.md needs a new rule.
@keithharvey
keithharvey force-pushed the sharing/01-foundations branch from 22dc1bd to fb30fe1 Compare July 22, 2026 20:59
@keithharvey

Copy link
Copy Markdown
Collaborator Author

Superseded by the modules stack: sharing now ships as one de-noised PR — #8463 (modules — multiplayer 2/2: sharing v2), tracked in #8412. Closing this chain; the branch stays for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant