Playerimpl units unrest

[scamiv]

If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)

Description:

Describe the PR.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

DISCORD_USERNAME

[coderabbitai]

Walkthrough

The PR refactors the units() method API from variadic spread parameters to explicit overloads supporting 0–3 individual unit types or a single array argument. This change is applied across interfaces, implementations, views, and call sites to improve type safety and consistency.

Changes

Cohort / File(s)	Summary
API Interface Definitions src/core/game/Game.ts	Updated Player and Game interface signatures from variadic units(...types: UnitType[]) to multiple overloads: no-arg, single type, two types, three types, and array-of-types variants.
Core Implementation src/core/game/PlayerImpl.ts, src/core/game/GameImpl.ts	Replaced variadic units method with overloaded signatures and unified implementation. Added optimized dispatch logic for 0–3 type arguments and array inputs to improve performance and type safety.
View Layer src/core/game/GameView.ts	Implemented overloaded units methods for both PlayerView and GameView with corresponding unified implementations that filter by owner and type constraints; added specialized paths for small-arity inputs.
Call Site Updates src/client/graphics/PlayerIcons.ts, src/core/execution/nation/NationStructureBehavior.ts	Changed unit retrieval calls from spread syntax game.units(...nukeTypes) to direct array syntax game.units(nukeTypes) to match new overload signatures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🔧 From variadic spreads to overloads we go,
Type safety blooms where once ambiguity grew.
Three types or one, an array—all paths flow,
Each call now crystal clear, precisely true.
The units gather, filtered and refined,
A cleaner API, with performance in mind. ✨

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description contains only repository template placeholders with no actual information about the changes, linked issues, or completed checklist items.	Provide a concrete description explaining the refactoring of the units() API from variadic to explicit overloads, why this change improves the codebase, and complete the provided checklist items.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Playerimpl units unrest' is vague and unclear, using non-descriptive terms that don't meaningfully convey what was changed.	Use a clear, descriptive title like 'Refactor units() method to use explicit overloads instead of variadic parameters' to better communicate the change.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/core/game/PlayerImpl.ts (1)

216-233: Returning internal _units array directly for no-args / empty-array case.

When called with no arguments (line 227) or an empty array (line 233), units() returns the raw this._units reference. Callers that mutate the returned array (e.g., splice, push) would corrupt internal state. This is likely pre-existing behavior, but worth noting: the empty-array path (len === 0) now also exposes the raw reference, matching the no-args path.

Current callers appear safe (they chain .filter() / .sort() which create new arrays), so this is not a blocker.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/game/PlayerImpl.ts` around lines 216 - 233, The units method
currently returns the internal this._units array directly when called with no
arguments or an empty array, exposing internal state to mutation; update
PlayerImpl.units so both the no-args path and the empty-array branch return a
shallow copy of this._units (e.g., using slice or spread) instead of the raw
array to prevent callers from mutating internal state; ensure the change is
applied inside the units(...) function around the early returns for a ===
undefined and len === 0 while keeping all other type-filtering behavior
unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/core/game/PlayerImpl.ts`:
- Around line 216-233: The units method currently returns the internal
this._units array directly when called with no arguments or an empty array,
exposing internal state to mutation; update PlayerImpl.units so both the no-args
path and the empty-array branch return a shallow copy of this._units (e.g.,
using slice or spread) instead of the raw array to prevent callers from mutating
internal state; ensure the change is applied inside the units(...) function
around the early returns for a === undefined and len === 0 while keeping all
other type-filtering behavior unchanged.