Skip to content

fix: remove Lua-mod fiction, realign on save-file-injection (ADR-001) model#34

Merged
SolshineCode merged 1 commit into
mainfrom
fix/remove-lua-fiction
May 12, 2026
Merged

fix: remove Lua-mod fiction, realign on save-file-injection (ADR-001) model#34
SolshineCode merged 1 commit into
mainfrom
fix/remove-lua-fiction

Conversation

@SolshineCode
Copy link
Copy Markdown
Owner

Summary

2026-05-11 Unciv research (docs/unciv-research-2026-05-11.md) found that the 2026-05-08 sprint's mod.lua and bespoke scenario JSONs were architectural fiction — Unciv does not support Lua mod scripting or runtime hooks. This PR removes the fiction, reframes the bridge↔Unciv contract on ADR-001's actual save-file-injection model, and honestly demotes the audit claims that depended on the fiction.

What's deleted

  • mod/ClaudeKingdoms/mod.lua
  • mod/ClaudeKingdoms/scenarios/Tutorial/ + LongRunningCampaign/
  • bridge/tests/test_integration.py (was a Python simulation of mod.lua)
  • 7 scenario-JSON-shape test cases in test_tutorial_flow.py

What's reframed

  • bridge/tutorial.py: scenario_to_loadscenario_stage (returns stage id, not a deleted filename)
  • docs/bridge-tui-schema.md: per-city payload is for an external save-edit tool, not mod.lua
  • mod/ClaudeKingdoms/README.md: documents the real integration model + schema warnings
  • CONTRIBUTING.md: contribution path feat(spike): implement save/load hook feasibility #3 + test ownership + code style cleaned of Lua refs
  • docs/audit-2026-05-08.md: 2026-05-11 retraction header listing every claim that demotes
  • .claude/handoff.md: full rewrite — honest acceptance-criteria status (10 ✅ / 5 🟡 / 6 ❌ vs prior over-stated 18 ✅)

What's preserved

The Python bridge, TUI, mod JSON content (Nations / Buildings / Beliefs / Tutorials / ModOptions), onboarding flow, frontier helper, audit log, daemon. None of those changed behavior.

Test plan

  • pytest -q → 248 passed (was 262; -14 from removed fictional tests, no real behavior changes)
  • grep -rn "mod.lua\|registerHook" returns only the retraction-doc references

… model

Background: 2026-05-08 supervision sprint shipped a 350-line mod.lua and
bespoke scenario.json/Map.json files for Unciv. Subsequent research
(docs/unciv-research-2026-05-11.md) found that Unciv does not support
Lua mod scripting or runtime hooks, and that scenarios are saved-game
files in a scenarios/ folder — not JSON manifests. Both were
architectural fiction that Unciv would not have loaded.

Removed:
- mod/ClaudeKingdoms/mod.lua (350 lines, fictional Lua hook handlers)
- mod/ClaudeKingdoms/scenarios/Tutorial/ (scenario.json + Map.json — wrong shape)
- mod/ClaudeKingdoms/scenarios/LongRunningCampaign/ (same)
- bridge/tests/test_integration.py (Python simulation of the fictional
  mod.lua's turn-end hook — the contract it tested doesn't exist)
- 7 cases in bridge/tests/test_tutorial_flow.py that asserted the
  deleted scenario JSON files' shape

Renamed / reframed (bridge code that's still sound):
- bridge/tutorial.py: scenario_to_load → scenario_stage. Returns
  a stage identifier ("tutorial" / "campaign") instead of a deleted
  filename. Old name kept as a back-compat alias.
- bridge/tests/test_per_city_status.py: dropped the Lua-glyph-table
  test name in favor of a generic downstream-consumer contract test.
  The bridge-side calculation itself is unchanged and correct.

Docs updates:
- mod/ClaudeKingdoms/README.md: documents the actual integration model
  (pure JSON content + future external save-edit tool, ADR-001's path).
  Adds explicit schema warnings: only Buildings.json and Units.json
  have "proper schema" per Unciv docs — others need real-parser
  validation.
- CONTRIBUTING.md: contribution path #3 reworded; Lua mention removed
  from test ownership matrix; Lua code-style section removed.
- docs/bridge-tui-schema.md: per_city_rewards description corrected —
  it's what an external save-edit tool would consume, not what mod.lua
  would read at runtime.
- docs/audit-2026-05-08.md: prominent 2026-05-11 retraction header
  listing every audit claim that demotes on this finding.
- .claude/handoff.md: full rewrite. Drops "V1 complete" claim,
  surfaces the honest acceptance-criteria status (10 ✅ + 5 🟡 + 6 ❌
  vs the prior over-stated 18 ✅), and documents the save-edit tool
  as the critical-path next deliverable.

Added:
- docs/unciv-research-2026-05-11.md: the source-cited Comet research
  that triggered this cleanup.
- .gitignore: logs/ (the daemon writes audit logs there; not for VCS).

Tests: 248 passing (was 262; net -14 from removed fictional tests).
No bridge-side behavior changed.

The Python bridge, TUI, mod JSON content, and onboarding flow are
still real and tested. The bridge↔Unciv integration leg is now
honestly missing rather than dishonestly claimed — it requires an
external save-edit tool against Unciv's actual save format, which is
the project's critical-path next deliverable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SolshineCode SolshineCode merged commit ea28db1 into main May 12, 2026
1 check passed
@SolshineCode SolshineCode deleted the fix/remove-lua-fiction branch May 12, 2026 00:59
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request performs a significant architectural correction by removing fictional Lua-based mod scripting and scenario JSON manifests that are not supported by the Unciv engine. It introduces a research document clarifying that Unciv integration must occur via offline save-file injection and updates the bridge, documentation, and tests to reflect this model. Key feedback includes a correction to a .gitignore rule that inadvertently un-ignored .egg-info directories and a recommendation to update test assertions to use the new scenario_stage API instead of its deprecated alias.

Comment thread .gitignore
dist/
build/
*.egg-info/ No newline at end of file
*.egg-info/logs/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This change replaces the ignore rule for *.egg-info/ directories with *.egg-info/logs/. This is likely a mistake as it stops ignoring the .egg-info directories themselves, only ignoring a logs subdirectory within them. If the goal was to ignore a top-level logs/ directory, it should be added as a separate line.

*.egg-info/
logs/

def test_scenario_stage_returns_tutorial_when_not_yet_done():
flow = TutorialFlow()
assert scenario_to_load(flow) == "Tutorial"
assert scenario_to_load(flow) == "tutorial"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test function has been renamed to test_scenario_stage_... but the assertion still calls the deprecated scenario_to_load alias. To maintain consistency with the new API and ensure the new function is tested directly, please update these calls (lines 159, 161, 168, 174) to use scenario_stage. Note that the import on line 24 will also need to be updated.

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