refactor: extract goal progression controller from main.gd (#177)

[itsmiso-ai]

Fixes #177

Extract pure game logic for goal progression into a new GoalProgression class (scripts/goal_progression.gd). This is the first domain controller extracted from main.gd, reducing its size and making goal-related logic testable without UI-heavy Control instantiation.

Changes

• New scripts/goal_progression.gd — class_name GoalProgression with four static methods:

  • init_goals(completed_ids) — selects the next active goal from the catalog
  • compute_progress(goal, game_state) — updates progress for all goal types
  • check_and_rotate(goal, completed_ids) — detects completion and rotates to the next goal
  • process_tick(goal, completed_ids, game_state) — convenience wrapper combining compute + rotate

• scripts/main.gd — replaces inline goal logic with calls to GoalProgression:

  • bootstrap_state() → GoalProgression.init_goals(completed_goal_ids)
  • _on_tick() → GoalProgression.process_tick(...)

Why this matters

main.gd (2561 lines) mixes UI rendering, game simulation, and domain logic. Extracting goal progression into a standalone class:

• Eliminates the need for scene-node mocking in goal-related tests
• Makes the tick loop cleaner and easier to reason about
• Follows the single-responsibility pattern established by RotatingGoal (data) and LayoutMath (pure math)

Next steps

Remaining domains to extract from main.gd:

• Worker cap / recruit / upkeep logic
• Task selection (choose_task, tasks_for_kind, task_distance)
• Resource reservation/release
• Build placement validation

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: review@https://litellm.jory.dev/v1 (openai)

Recommendation

Approve. This PR successfully extracts the goal progression logic from the monolithic main.gd into a dedicated, pure-logic controller, fulfilling the primary objective of issue PR 177.

Change-by-Change Findings

scripts/goal_progression.gd (New)

• Logic Extraction: Successfully implements a stateless (static method based) controller for goal lifecycle management.
• Responsibility Segregation: The class handles initialization, progress computation (via RotatingGoal), and rotation logic without any dependency on Godot's Node or Control classes. This directly enables unit testing without heavy UI mocking.
• API Design: The process_tick method provides a clean, structured dictionary result containing the new goal, updated completion lists, and completion status, which simplifies the caller's logic.

scripts/main.gd (Modified)

• Refactoring: Replaces complex inline goal logic with calls to the new GoalProgression controller.
• State Management: Correctly updates active_goal and completed_goal_ids using the results returned by the controller, ensuring state consistency.
• Event Triggering: Maintains existing functionality by triggering the completion event based on the was_completed flag from the controller.

Standards Compliance

• Single Responsibility: Follows the pattern established by RotatingGoal and LayoutMath by separating domain logic from the main coordination loop.
• Testability: By removing UI dependencies from the goal logic, this change significantly lowers the barrier for writing automated tests for game progression.
• Code Cleanliness: Reduces the line count and cognitive load of main.gd (which was noted as being over 2500 lines).

Linked Issue Fit

• Issue PR 177: This PR directly addresses the first priority of the tech debt audit: "Extract domain controllers from main.gd: start with goal progression... so tests do not need UI-heavy Control instantiation."

Unknowns or Needs Verification

• Regression Risk: While the logic is extracted, the core simulation loop in main.gd is still modified. A smoke test of the game loop and save/load cycle is recommended to ensure the active_goal state persists correctly through the new GoalProgression.init_goals flow.

[joryirving]

CI failing

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: review@https://litellm.jory.dev/v1 (openai)

Recommendation

Approved. This PR successfully extracts the goal progression logic from the monolithic main.gd into a dedicated, testable, and pure logic controller GoalProgression, fulfilling the requirements of issue PR 177.

Change-by-Change Findings

scripts/goal_progression.gd (New File)

• Implementation: Created a new GoalProgression class with static methods for initialization, progress computation, and rotation logic.
• Design: The class is designed as a pure logic controller that operates on Dictionary types (goal, game_state), ensuring it does not depend on Godot's UI tree (Control nodes). This directly addresses the technical debt noted in the linked issue regarding the difficulty of testing main.gd without heavy UI mocking.
• Consistency: The use of a result Dictionary in process_tick to return updated state (active_goal, completed_ids) and metadata (was_completed, goal_id) is a clean way to handle state transitions in a functional style.

scripts/main.gd (Modified)

• Refactoring: Replaced inline goal logic in bootstrap_state() and _on_tick() with calls to the new GoalProgression controller.
• State Management: Correctly updates local active_goal and completed_goal_ids based on the returned dictionary from GoalProgression.process_tick.
• UI/Events: Maintains the existing behavior of triggering a push_event when a goal is completed, ensuring no regression in user feedback.

Linked Issue Fit

• Requirement Match: The PR directly implements the first step of the P1 recommendation in issue PR 177: "Extract domain controllers from main.gd: start with goal progression".
• Goal Achievement: By moving this logic out of main.gd, the codebase moves closer to the goal of reducing the size and complexity of the main coordination script, making the domain logic testable in isolation.

Standards Compliance

• Repository Conventions: The implementation follows the established pattern of extracting pure math/logic into standalone modules (similar to LayoutMath and RotatingGoal).
• Code Quality: The new class uses class_name for global access and follows the project's preference for explicit behavior over silent state changes.

Unknowns or Needs Verification

• Testing: While the PR enables easier unit testing for goal progression, no new test files were included in this PR. It is recommended to verify that existing integration tests still pass and to consider adding unit tests for scripts/goal_progression.gd to leverage its new testability.

[itsmiso-ai]

CI checks are now passing on the latest commit. All four checks (Headless smoke test, Script test suite, macOS validation, review) completed successfully.

@joryirving Could you please dismiss your earlier changes-requested review? The CI issue has been resolved.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Summary

Approve. This PR extracts goal progression logic from main.gd into a standalone GoalProgression class with 14 deterministic headless tests. It directly addresses issue PR 177's first priority: "start with goal progression."

Change-by-Change Findings

scripts/goal_progression.gd (new, 71 lines)

• Design: Correct static class with four methods matching the responsibilities described in the header docstring.
• init_goals() delegates to RotatingGoal.select_next_active_goal() — clean passthrough.
• compute_progress() delegates to three RotatingGoal compute methods — correct aggregation.
• check_and_rotate() handles both the no-op path (incomplete goal returns unchanged result) and the completion path (appends ID, rotates). Returns a structured dict with active_goal, completed_ids, was_completed, and goal_id.
• process_tick() is a convenience wrapper that calls compute then rotate, matching the original _on_tick flow.
• Empty-goal guard clauses are present in both compute_progress and check_and_rotate.

scripts/main.gd (modified, +8/-14)

• Added GoalProgression preload alongside existing RotatingGoal preload.
• bootstrap_state(): replaced RotatingGoal.select_next_active_goal(completed_goal_ids) with GoalProgression.init_goals(completed_goal_ids) — semantically equivalent.
• _on_tick(): replaced 7 lines of inline goal logic with a single GoalProgression.process_tick() call and result unpacking. The push_event("Goal completed: ...") message is preserved.
• Behavioral equivalence confirmed: original flow was compute three progress types → check completion → rotate if done. New flow does the same through process_tick.

tests/test_goal_progression.gd (new, 239 lines)

• 14 test cases covering all four public methods.
• Test pattern matches existing repo tests (extends SceneTree, manual pass/fail counting, assert in integration test).
• Tests cover: init with empty/completed/all-done states; compute for resource and build goal types; rotation detection, ID appending, and was_completed flag; full process_tick flow; and multi-goal sequential rotation.
• No DisplayServer or scene-node dependency — fully deterministic headless run.

Standards Compliance

• Single-responsibility: GoalProgression owns goal lifecycle; RotatingGoal owns goal data model and catalog operations. Clean separation.
• Pure logic extraction: Class has no UI dependencies, no Control instantiation, no signals. Follows the pattern established by LayoutMath (pure math) and RotatingGoal (data).
• No behavioral change: Original _on_tick goal logic is replaced 1:1. The event message is preserved.
• Test coverage: 14 cases for a 71-line class is thorough. Existing repo tests for RotatingGoal remain valid.

Linked Issue Fit

Issue PR 177 explicitly calls out "start with goal progression" as the first extraction target. The PR body enumerates the exact four methods extracted and lists remaining domains to extract (worker cap, task selection, resource reservation, build placement). This is a clean, scoped first step.

Unknowns / Needs Verification

None identified. Tool harness errors were argument-missing issues in the harness config, not failures of the PR. No evidence providers were configured for this review.

[itsmiso-ai]

CI is now passing — all checks are green:

• ✅ review (AI PR Review)
• ✅ Headless smoke test
• ✅ Script test suite
• ✅ macOS validation

The test commit (80eaeb1) added 14 deterministic headless tests for GoalProgression. Please re-review when you get a chance.