Skip to content

Comments

Small refactor/cleanup: RadialMenuElements and PlayerImpl#3239

Merged
jrouillard merged 1 commit intomainfrom
small-cleanup
Feb 19, 2026
Merged

Small refactor/cleanup: RadialMenuElements and PlayerImpl#3239
jrouillard merged 1 commit intomainfrom
small-cleanup

Conversation

@VariableVince
Copy link
Contributor

Description:

PR 7/x in effort to break up PR #3220. Follows on already merged #3238.

Please see if these can be merged for v30.

-RadialMenuElements:

  • getAllEnabledUnits: use camelCase instead of PascalCase so change Units into units.
  • getAllEnabledUnits: use StructureTypes to loop through instead of having 6 individual lines of code to check if a structure is enabled. StructureTypes contains and will keep containing the same structures as those that are checked here. PR 3220 will later on also replace the individual lines for the attack type units into a loop with a newly introduced Types array, in the same way as we do in this PR with StructureTypes.
  • getAllEnabledUnits: rename the long named const addStructureIfEnabled to just addIfEnabled, which is clear enough from the context it is used in.

-PlayerImpl

  • buildableUnits: removed unnecessary "as BuildableUnit" after the in-loop return; the function itself already says it returns BuildableUnit[].

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:

tryout33

@VariableVince VariableVince added this to the v30 milestone Feb 19, 2026
@VariableVince VariableVince self-assigned this Feb 19, 2026
@VariableVince VariableVince requested a review from a team as a code owner February 19, 2026 00:21
@VariableVince VariableVince added the Refactor Code cleanup, technical debt, refactoring, and architecture improvements. label Feb 19, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

No actionable comments were generated in the recent review. 🎉


Walkthrough

Two targeted refactorings modernize the codebase: RadialMenuElements.ts replaces explicit per-type unit enabling with a generic StructureTypes-based iteration, while PlayerImpl.ts removes an unnecessary explicit type assertion, relying instead on type inference.

Changes

Cohort / File(s) Summary
Generic Unit Enabling Refactor
src/client/graphics/layers/RadialMenuElements.ts
Replaces individual type-specific unit enabling calls (City, DefensePost, Port, MissileSilo, SAMLauncher, Factory) with a single generic iteration over StructureTypes. Renames internal helper from addStructureIfEnabled to addIfEnabled. Updates imports to include StructureTypes.
Type Assertion Cleanup
src/core/game/PlayerImpl.ts
Removes explicit type cast "as BuildableUnit" in buildableUnits method; returns object directly with type inference. No runtime behavior change.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🎭 From many types to patterns clean,
One loop replaces what has been,
The casts now fade, the types align,
Where inference does the work just fine! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring changes across two files: RadialMenuElements and PlayerImpl, with clear focus on cleanup.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the specific refactoring goals for each file modified.

✏️ 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

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

@github-project-automation github-project-automation bot moved this from Triage to Final Review in OpenFront Release Management Feb 19, 2026
@jrouillard jrouillard added this pull request to the merge queue Feb 19, 2026
Merged via the queue into main with commit 2a7db43 Feb 19, 2026
14 checks passed
@jrouillard jrouillard deleted the small-cleanup branch February 19, 2026 00:29
@github-project-automation github-project-automation bot moved this from Final Review to Complete in OpenFront Release Management Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Code cleanup, technical debt, refactoring, and architecture improvements.

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

2 participants