Skip to content

refactor: bulletproof step 2 - domain layer - #8097

Open
Rathoz wants to merge 11 commits into
mainfrom
bulletproof-2
Open

refactor: bulletproof step 2 - domain layer#8097
Rathoz wants to merge 11 commits into
mainfrom
bulletproof-2

Conversation

@Rathoz

@Rathoz Rathoz commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Make a domain layer for matches, brackets and team history

How did you test this change

Rathoz and others added 4 commits August 21, 2026 09:46
@Rathoz
refactor: bulletproof step 1.M
46c1a35
@Rathoz
Merge branch 'main' into bulletproof-1.M
0ef9b74
@Rathoz
refactor: bulletproof step 2
c1b9341
@Rathoz @claude
Merge branch 'main' into bulletproof-1.M
57c6d7e 
Conflict in lua/wikis/commons/MatchGroup/Util.lua: main changed three things
in code this branch moved out. Kept the slim re-export version and migrated
each change to its new home:

- ---@field roles string[]? (#8016) -> standardPlayer in Util/Types.lua
- ---@field submatches MatchGroupUtilSubgroup[] (#7978) -> MatchGroupUtilMatch
  in Util/Types.lua
- match.submatches = groupBySubgroup(match) (#7978) -> MatchUtil.matchFromRecord
  in Util/Match.lua, via MatchUtil rather than the re-export

That last one is load bearing: Util/Starcraft.lua was changed in #7978 to read
match.submatches instead of recomputing it, so dropping the assignment would
have left starcraft submatches empty.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rathoz
Rathoz added this pull request to stack #8098 September 16, 2026 12:04
mbergen
mbergen reviewed Sep 16, 2026
View reviewed changes
Comment thread lua/spec/domain/bracket_spec.lua
@Rathoz @claude
Merge branch 'bulletproof-1.M' into bulletproof-2
d51ab4a 
Resolves a conflict in MatchGroup/Util/Types.lua, where upstream edited two
annotations in place while this branch moved them into the domain layer.
Both upstream additions are carried into Domain/Match/Model.lua:

- standardPlayer gained `roles string[]?`
- MatchGroupUtilMatch gained `submatches MatchGroupUtilSubgroup[]`

Also repoints two references the earlier moves missed, because neither is an
import: the perf profiling list in Bracket/Template.lua, and stale module paths
in the MatchGroup/Util header comment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Base automatically changed from bulletproof-1.M to main September 18, 2026 08:29
Rathoz and others added 2 commits September 18, 2026 11:42
@Rathoz @claude
Merge branch 'main' into bulletproof-2
1adea28 
bulletproof-1.M was squash merged to main (#7999), so git replays its
changes from a merge base that predates them. Two kinds of fallout:

Review fixes from the tail of #7999 (ee639cf) landed on main after this
branch had already merged 1.M, so they are carried over here. Where step 2
moved the code, they are applied at the new location:

  MatchGroup/Util/Bracket.lua -> Domain/Bracket/Model.lua
    nilIfEmpty -> String.isNotEmpty in computeLowerMatchIdsFromLegacy
    MatchGroupUtilMatchCoordinates fields number -> integer
  MatchGroup/Util/Match.lua -> Domain/Match/Model.lua
    NOW from Date.getCurrentTimestamp rather than os.time
    createOpponent defaults to Opponent.literal
    playerFromRecord takes a match2player
  MatchGroup/Display/Helper.lua, MatchGroup/Util/Types.lua,
  spec/match_group_util_spec.lua: applied in place

Domain must not reach for per wiki code, so the Opponent import is
Module:Opponent rather than Module:Opponent/Custom. Opponent.literal is
the same constant either way.

The squash also re-added four files that step 2 had moved, because from
the merge base they look like additions on main alone. Removed again:
MatchGroup/Util/{Bracket,Match}.lua and their specs, which live on as
Domain/{Bracket,Match}/Model.lua and spec/domain_{bracket,match}_spec.lua.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rathoz
fix annos
203fcac
@Rathoz
Rathoz marked this pull request as ready for review September 18, 2026 09:16
@Rathoz
Rathoz requested review from a team as code owners September 18, 2026 09:16
Copilot AI lite review requested due to automatic review settings September 18, 2026 09:16
Copilot AI reviewed Sep 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical caller migration issue can drop grand-final reset data; the workflow also has an action-version inconsistency.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Refactors match, bracket, and team-history logic into domain modules, updates consumers and tests, and adds import-layer CI validation.

Changes:

  • Adds domain models and stores.
  • Updates match-summary, display, and squad-history callers.
  • Adds import-layer checking and related tests.
File summaries
File Summary
scripts/check_lua_imports.py Adds import-layer validation.
lua/wikis/warcraft/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/warcraft/GameSummary.lua Updates game-summary integration.
lua/wikis/underlords/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/trackmania/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/thefinals/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/thefinals/GameSummary.lua Updates game-summary integration.
lua/wikis/tft/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/tarkovarena/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/pubgmobile/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/pubg/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/naraka/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/lab/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/hearthstone/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/halo/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/geoguessr/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/geoguessr/GameSummary.lua Updates game-summary integration.
lua/wikis/freefire/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/fortnite/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/deltaforce/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/counterstrike/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/commons/PrizePool/Import.lua Updates domain coordinate imports.
lua/wikis/commons/MatchSummary/Starcraft/Ffa.lua Updates match-summary integration.
lua/wikis/commons/MatchSummary/Base.lua Updates match retrieval.
lua/wikis/commons/MatchGroup/Util/Types.lua Updates utility type definitions.
lua/wikis/commons/MatchGroup/Util.lua Re-exports domain APIs and reset lookup.
lua/wikis/commons/MatchGroup/Display/SingleMatch.lua Updates display API usage.
lua/wikis/commons/MatchGroup/Display/SingleGame.lua Updates display API usage.
lua/wikis/commons/MatchGroup/Display/Horizontallist.lua Updates display API usage.
lua/wikis/commons/MatchGroup/Display/Bracket.lua Updates display API usage.
lua/wikis/commons/GameSummary/Starcraft.lua Updates game-summary integration.
lua/wikis/commons/GameSummary.lua Updates game-summary integration.
lua/wikis/commons/Features/Squad/Types.lua Updates squad type definitions.
lua/wikis/commons/Features/Squad/Lib/History.lua Uses domain team history.
lua/wikis/commons/Features/Squad/Auto.lua Uses domain team history.
lua/wikis/commons/Domain/TeamHistory/Store.lua Adds team-history storage.
lua/wikis/commons/Domain/TeamHistory/Model.lua Adds team-history model.
lua/wikis/commons/Domain/Match/Store.lua Adds match storage.
lua/wikis/commons/Domain/Match/Model.lua Adds match model.
lua/wikis/commons/Domain/Bracket/Model.lua Adds bracket model.
lua/wikis/commons/Domain/Bracket/Coordinates.lua Adds bracket coordinates.
lua/wikis/commons/Bracket/Template.lua Updates coordinate integration.
lua/wikis/callofduty/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/autochess/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/arenafps/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/apexlegends/MatchSummary/Ffa.lua Updates match-summary integration.
lua/wikis/ageofempires/MatchSummary/Ffa.lua Updates match-summary integration.
lua/spec/squad_history_spec.lua Updates squad-history tests.
lua/spec/squad_auto_spec.lua Updates squad automation tests.
lua/spec/match_group_util_spec.lua Updates utility API tests.
lua/spec/domain_team_history_spec.lua Adds team-history domain tests.
lua/spec/domain_match_spec.lua Adds match domain tests.
lua/spec/domain_bracket_spec.lua Adds bracket domain tests.
.github/workflows/luacheck.yml Adds import checks to CI.
Review details

Suppressed comments (1)

.github/workflows/luacheck.yml:46

  • This new job pins actions/setup-python to v6, while the repository's other workflows (and the later Python setup in this same workflow) use v7. Please use the repository-wide v7 pin here as well so the import-rule check does not run on a divergent action version.
        uses: actions/setup-python@v6
  • Files reviewed: 54/54 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lua/wikis/commons/MatchGroup/Util.lua
ElectricalBoy
ElectricalBoy reviewed Sep 19, 2026
View reviewed changes
Comment thread .github/workflows/luacheck.yml Outdated
Comment thread .github/workflows/luacheck.yml
Comment thread scripts/check_lua_imports.py Outdated
Rathoz and others added 2 commits October 5, 2026 13:47
@Rathoz @claude
ci: annotate import rule violations on the diff
0cc5f2a 
The import check only wrote to the raw job log, so a violation was something
you had to go looking for. Each one now carries the line and column of the
import that broke the rule, and is printed as a workflow command on GitHub so
it lands on the diff, the way the LuaLS job already does. Anywhere else it
stays plain text, on stderr.

Also brings the job in line with the other workflows: setup-python v7 pinned
to 3.14, and PYTHONUNBUFFERED so the output is not held back from the log.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rathoz @claude
test: move the domain specs into spec/domain
d661c4c 
The spec folder is flat at 77 files. The domain specs already spelled the
hierarchy out in their names, so they become spec/domain/<entity>_spec.lua.
The luacheck pattern that grants specs their test globals had to learn to
cross a folder boundary to follow them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rathoz
Rathoz requested a review from ElectricalBoy October 5, 2026 11:49
Rathoz and others added 2 commits October 5, 2026 17:00
@Rathoz @claude
Merge branch 'main' into bulletproof-2
acda3a9 
Nothing conflicted this time: main did not touch any of the files this branch
moved into Domain, and none of the new code imports the old paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Rathoz @claude
Merge branch 'main' into bulletproof-2
28b8834 
One conflict, the same shape as last time: upstream keeps editing the
annotations in MatchGroup/Util/Types.lua while this branch has moved them to
Domain. #8183 corrected two of them, and they are carried over to where they
now live, in Domain/Bracket/Model.lua:

  MatchGroupUtilMatchlistBracketData.matchId -> matchIndex, which is the field
    MatchGroup/Display/Horizontallist actually reads
  MatchGroupUtilMatchlist.bracketDatasById holds matchlist bracket data, not
    bracket bracket data

Nothing else upstream touched a relocated file, and nothing new imports the old
paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbergen mbergen mbergen left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ElectricalBoy ElectricalBoy Awaiting requested review from ElectricalBoy

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

c: match2 c: squad

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Rathoz @ElectricalBoy @mbergen