Refactor: decompose GameService god class, eliminate duplication across controllers and services

[Copilot]

GameService.java was a 930-line god class handling AI, combat, policy, movement, capture, and win conditions. Controllers and services had duplicated validation, error handling, and HTTP header construction.

GameService decomposition (930 → 330 lines)

• AiService — AI opponent decision-making (threat detection, target selection, garrison logic)
• CombatService — Combat resolution with morale modifiers, army merging
• PolicyService — Stat recovery, desertion, soldier generation, ruler stats, income calculation
• GameService remains the tick orchestrator and public API facade; all existing tests work unchanged since extracted classes are package-private POJOs (tests construct new GameService() directly)

DRY consolidation

• GameController: extracted validateDecision() + executePolicyChange() — two decision endpoints shared identical validation and error mapping
• BackendService: extracted jsonEntity(), sessionEntity(), sessionJsonEntity() — 10+ methods repeated header construction
• WebController: extracted executeWithBackendErrorHandling(Supplier<GameState>) — two decision proxy endpoints had identical try/catch chains
• GameService: extracted TileOccupancy determineTileOccupancy(x, y) — village and castle capture duplicated the same army-scanning loop
• Moved createSnapshot() to GameState — the data class owns its own deep-copy logic

Thread safety fix

• Added executeWithSessionState(GameState, Function<GameService, T>) to GameService — atomically sets the session's game state and executes the action under the same synchronized lock, preventing interleaved requests from operating on the wrong session's state
• Updated all 6 session endpoints in GameController to use executeWithSessionState() instead of separate setGameState() + operation calls

Naming improvements

• army1/army2 → attacker/defender, ax/ay → armyX/armyY
• Magic numbers → named constants: THREAT_DETECTION_RANGE, CAPTURE_DANGER_RANGE, VILLAGE_ATTACK_FORCE_RATIO, POLICY_COOLDOWN_TICKS
• executeAI() → executeAiTurn(), makeAIDecision() → assignAiTarget()

Cleanup

• Removed redundant comments that restated code (field comments, inline // Default, // Place initial armies)
• Fixed fully-qualified inline references (org.springframework.web.server.ResponseStatusException) → proper imports
• Replaced wildcard imports with explicit imports
• Removed unused MORALE_BASELINE constant from CombatService
• Replaced O(tickCount) incrementTick() loop in createSnapshot() with direct setTickCount() assignment
• Fixed constant-folded enemy player ID derivation in AiService.findCastleAssaultTarget() to use army.getPlayerId() for future-proof flexibility

All 137 tests pass, both modules compile. No public API contracts changed.

[Copilot]

Pull request overview

Refactors the backend game loop by decomposing the former “god” GameService into focused internal services (AI, combat, policy) and reduces duplicated request/response boilerplate across backend + web-client controllers/services, while keeping public API behavior intact.

Changes:

• Split game logic into AiService, CombatService, and PolicyService, with GameService as the orchestrator/facade.
• Consolidated duplicated validation/error-handling in GameController and proxy error-handling in WebController.
• Centralized repeated header/entity construction in web-client BackendService, and moved snapshot/deep-copy logic into GameState#createSnapshot().

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
web-client/src/main/java/com/barony/webclient/service/BackendService.java	Adds reusable JSON/session HttpEntity helpers and expands session-aware API methods.
web-client/src/main/java/com/barony/webclient/controller/WebController.java	Extracts shared backend error-handling wrapper and adds session decision endpoint handling via it.
backend/src/main/java/com/barony/backend/service/SessionService.java	Minor cleanup/formatting (no behavioral refactor in shown diff).
backend/src/main/java/com/barony/backend/service/PolicyService.java	New package-private service encapsulating policy-related tick effects and stats/income calculations.
backend/src/main/java/com/barony/backend/service/MapGenerator.java	Removes redundant comments/docs (no functional change).
backend/src/main/java/com/barony/backend/service/GameService.java	Becomes the orchestrator; delegates to new services; adds capture occupancy helper; uses GameState#createSnapshot().
backend/src/main/java/com/barony/backend/service/CombatService.java	New package-private service for combat resolution and friendly-army merging.
backend/src/main/java/com/barony/backend/service/AiService.java	New package-private service for AI turn logic and garrison behavior.
backend/src/main/java/com/barony/backend/model/Tile.java	Removes inline explanatory comments (no functional change).
backend/src/main/java/com/barony/backend/model/RulerDecision.java	Removes inline explanatory comments (no functional change).
backend/src/main/java/com/barony/backend/model/GameState.java	Adds createSnapshot() to own deep-copy logic.
backend/src/main/java/com/barony/backend/model/Army.java	Removes inline explanatory comments (no functional change).
backend/src/main/java/com/barony/backend/controller/GameController.java	Extracts shared decision validation + policy-change execution helpers and reuses them for session/non-session endpoints.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

createSnapshot() copies tickCount by calling incrementTick() in a loop. Since GameState has Lombok @Setter, this can be replaced with a direct setTickCount(tickCount) assignment to avoid O(tickCount) snapshot creation as games run longer.

Suggested change

	for (int i = 0; i < tickCount; i++) {
	snapshot.incrementTick();
	}
	snapshot.setTickCount(tickCount);

[Copilot]

GameService is a singleton Spring bean whose mutable gameState reference is swapped via setGameState(). Session endpoints call setGameState(sessionState) and then call another synchronized method (e.g., tick() / executeCommand()), which allows another request to interleave between those calls and run the operation against the wrong session’s state. Consider making GameService stateless (pass GameState into operations), or add a single synchronized helper that sets the state and executes an action under the same lock, and update controllers to use it.

[Copilot]

MORALE_BASELINE is declared but never used. Either remove it, or incorporate it into calculateEffectiveStrength() (e.g., to make the intent of morale-as-percentage explicit) to avoid dead code.

[Copilot]

In findCastleAssaultTarget(), int enemyPlayerId = (AI_PLAYER_ID == 1) ? 2 : 1; is constant-folded (AI_PLAYER_ID is always 2), so this conditional adds noise without changing behavior. Consider replacing it with 1 (or derive from army.getPlayerId() if you want it to work for a different AI player in the future).

Suggested change

	int enemyPlayerId = (AI_PLAYER_ID == 1) ? 2 : 1;
	int enemyPlayerId = (army.getPlayerId() == 1) ? 2 : 1;

[dmccoystephenson]

@copilot You are a coding agent tasked with resolving all open comments and review threads
on this pull request. Work through every comment systematically and apply the
appropriate code changes.

---

Instructions

1. Discover all comments

• Read every review thread, inline comment, and general PR comment
• Include comments from: Copilot, human reviewers, and any other bots
• Do not skip comments marked as suggestions — apply them if they make sense

2. For each comment, do the following:

• Understand what is being requested or flagged
• Locate the relevant file(s) and line(s)
• Apply the fix directly in code
• If a Copilot suggestion diff is provided, apply it (or an equivalent fix)
• If the comment is ambiguous, use your best judgment and reply in the PR thread or review comment explaining what you did; only add a code comment when explicitly requested or when future maintainers would benefit from it
• After addressing a comment, reply to the thread summarising what you did and mark it as resolved if your environment supports it

3. Types of issues to handle:

• Code quality / style issues (naming, formatting, readability)
• Logic bugs or edge cases flagged by reviewers
• Security concerns (e.g. input validation, secrets, injection risks)
• Performance suggestions
• Missing or incomplete error handling
• Test coverage gaps
• Documentation / comment improvements
• Dependency or import issues

4. Do NOT:

• Dismiss or ignore any comment without acting on it — note that "acting" can also mean replying to the thread with a rationale (e.g., won't fix, already addressed elsewhere) and leaving the code unchanged when appropriate
• Leave TODO comments as a substitute for fixing the issue
• Break existing tests while addressing comments
• Change functionality beyond the scope of what the comment asks for

---

After making changes

• Ensure all existing tests still pass
• Add or update tests if a comment explicitly asks for them, or if a bug fix warrants it
• Double-check that every open thread has been addressed, replied to, and resolved

---

Context

• This PR may contain Copilot-generated review comments — treat these with the same priority as human comments
• If two comments conflict with each other, prefer the human reviewer's intent and document the conflict and resolution in the PR conversation or commit message, not in a code comment
• Commit changes in logical groups if there are many unrelated fixes

[Copilot]

@copilot You are a coding agent tasked with resolving all open comments and review threads
on this pull request. Work through every comment systematically and apply the
appropriate code changes....

All 4 review comments addressed in 8661b46:

1. GameState.java:44-46 — Replaced incrementTick() loop with snapshot.setTickCount(tickCount) to avoid O(tickCount) snapshot creation.
2. GameService.java:32-34 — Added executeWithSessionState(GameState, Function) that atomically sets state and executes the action under the same synchronized lock. Updated all 6 session endpoints in GameController to use it, eliminating the race condition between setGameState() and the subsequent operation.
3. CombatService.java:19 — Removed unused MORALE_BASELINE constant.
4. AiService.java:169 — Replaced (AI_PLAYER_ID == 1) ? 2 : 1 with (army.getPlayerId() == 1) ? 2 : 1 so enemy player derivation works for any AI player ID.

All 137 tests pass, both modules compile.