Enable the @typescript-eslint/no-unused-vars eslint rule #2130
Conversation
|
|
WalkthroughProject-wide lint-oriented edits rename unused parameters to underscore-prefixed forms and add ESLint suppressions. ESLint config switches to @typescript-eslint/no-unused-vars enforcement. One behavior change: TargetPlayerExecution.tick now always deactivates after a tick. PlayerTypeSchema is removed from Schemas. Tests simplified accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Engine as Game Engine
participant Exec as TargetPlayerExecution
participant World as Game World
Engine->>Exec: tick(_ticks)
Note over Exec: Attempt to target player
Exec->>World: find target by ID
alt target found
Exec->>World: set current target
else target missing
Exec->>Engine: log warning / keep state
end
rect rgba(200,230,255,0.3)
note right of Exec: Change
Exec->>Exec: this.active = false
end
Exec-->>Engine: return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks
✅ Passed checks (1 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/components/Difficulties.ts (1)
77-90: Remove unusedkingSkullconstant or add context for why it's preserved.The
kingSkullconstant is defined but never used in the switch statement. Per YAGNI (You Aren't Gonna Need It), unused code should be removed to keep the codebase clean and maintainable.If this constant is intended for near-term use (e.g., a planned "King" difficulty level), please add a comment explaining the context. Otherwise, remove it and add it back when actually needed.
Apply this diff to remove the unused constant:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const kingSkull = html`<svg - stroke="currentColor" - fill="currentColor" - stroke-width="0" - viewBox="0 0 512 512" - height="100%" - width="100%" - xmlns="http://www.w3.org/2000/svg" - > - <path - d="M92.406 13.02l-.164 156.353c3.064.507 6.208 1.38 9.39 2.627 36.496 14.306 74.214 22.435 111.864 25.473l43.402-60.416 42.317 58.906c36.808-4.127 72.566-12.502 105.967-24.09 3.754-1.302 7.368-2.18 10.818-2.6l1.523-156.252-75.82 95.552-34.084-95.55-53.724 103.74-53.722-103.74-35.442 95.55-72.32-95.55h-.006zm164.492 156.07l-28.636 39.86 28.634 39.86 28.637-39.86-28.635-39.86zM86.762 187.55c-2.173-.08-3.84.274-5.012.762-2.345.977-3.173 2.19-3.496 4.196-.645 4.01 2.825 14.35 23.03 21.36 41.7 14.468 84.262 23.748 126.778 26.833l-17.75-24.704c-38.773-3.285-77.69-11.775-115.5-26.596-3.197-1.253-5.877-1.77-8.05-1.85zm333.275.19c-2.156.052-5.048.512-8.728 1.79-33.582 11.65-69.487 20.215-106.523 24.646l-19.264 26.818c40.427-2.602 80.433-11.287 119.22-26.96 15.913-6.43 21.46-17.81 21.36-22.362-.052-2.276-.278-2.566-1.753-3.274-.738-.353-2.157-.71-4.313-.658zm-18.117 47.438c-42.5 15.87-86.26 23.856-130.262 25.117l-14.76 20.547-14.878-20.71c-44.985-1.745-89.98-10.23-133.905-24.306-12.78 28.51-18.94 61.14-19.603 93.44 37.52 17.497 62.135 39.817 75.556 64.63C177 417.8 179.282 443.62 174.184 467.98c7.72 5.007 16.126 9.144 24.98 12.432l5.557-47.89 18.563 2.154-5.935 51.156c9.57 2.21 19.443 3.53 29.377 3.982v-54.67h18.69v54.49c9.903-.638 19.705-2.128 29.155-4.484l-5.857-50.474 18.564-2.155 5.436 46.852c8.747-3.422 17.004-7.643 24.506-12.69-5.758-24.413-3.77-49.666 9.01-72.988 13.28-24.234 37.718-46 74.803-64.29-.62-33.526-6.687-66.122-19.113-94.23zm-266.733 47.006c34.602.23 68.407 12.236 101.358 36.867-46.604 33.147-129.794 34.372-108.29-36.755 2.315-.09 4.626-.127 6.933-.11zm242.825 0c2.307-.016 4.617.022 6.93.11 21.506 71.128-61.684 69.903-108.288 36.757 32.95-24.63 66.756-36.637 101.358-36.866zM255.164 332.14c11.77 21.725 19.193 43.452 25.367 65.178h-50.737c4.57-21.726 13.77-43.45 25.37-65.18z" - ></path> - </svg>`; -Or, if you plan to use this soon, add a comment explaining the intent:
// eslint-disable-next-line @typescript-eslint/no-unused-vars + // TODO: Preserved for upcoming "King" difficulty level (issue #XXXX) const kingSkull = html`<svg
🧹 Nitpick comments (9)
src/core/configuration/DevConfig.ts (1)
68-70: Remove dead code instead of suppressing the warning.The variable
oldCostis declared but never used (Line 70 that would use it is commented out). Rather than suppressing the linting warning with an eslint-disable comment, remove both the variable declaration and the suppression comment. This keeps the code clean and makes future changes clearer.Apply this diff:
unitInfo(type: UnitType): UnitInfo { const info = super.unitInfo(type); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const oldCost = info.cost; // info.cost = (p: Player) => oldCost(p) / 1000000000; return info; }If you need to preserve this code for future development work, consider moving it to a dedicated comment block explaining the intent, rather than leaving suppressed dead code in the implementation.
src/client/graphics/layers/StructureLayer.ts (1)
247-251: Drop the unused color instead of silencing lint
colorisn’t read anywhere, so this disable comment just papers over dead code. Please delete the variable and branch, keeping lint clean and the TypeScript tidy.Apply this diff:
- let color = unit.owner().borderColor(); - if (unit.type() === UnitType.Construction) { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - color = underConstructionColor; - }src/server/Master.ts (1)
296-299: Remove unusedsleephelper or move to utilities module.This function is not called anywhere. Instead of disabling the lint rule, either remove the function or move it to a shared utilities file if you expect to use it soon.
Apply this diff to remove the unused function:
-// eslint-disable-next-line @typescript-eslint/no-unused-vars -function sleep(ms: number): Promise<void> { - return new Promise((resolve) => setTimeout(resolve, ms)); -} -src/client/graphics/fx/SpriteFx.ts (1)
80-81: Consider removing the unused variable entirely.The variable
tis computed but never used. If it's not needed for future functionality or debugging, remove line 81 entirely rather than suppressing the lint warning.Apply this diff if the variable is truly unnecessary:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const t = this.elapsedTime / this.duration;If you're keeping it for debugging or future use, consider adding a comment explaining why.
src/client/graphics/layers/RailroadLayer.ts (1)
105-108: Drop unused x/y to avoid dead code.
game.x(...)andgame.y(...)return numbers without side effects, so creatingxandyand silencing the rule only adds noise. Please remove the variables (and the disable comments) so the loop stays lean.- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const x = this.game.x(railRoad.tile); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const y = this.game.y(railRoad.tile);src/core/game/TerrainSearchMap.ts (1)
18-34: Consider removing unused variables instead of disabling them.Lines 22-25 extract
shorelineandoceanfrom the packed byte but never use them. Rather than extracting and immediately suppressing with eslint-disable comments, consider not extracting these variables at all. This keeps the code cleaner and avoids the need for inline suppressions.Apply this diff to simplify:
node(x: number, y: number): SearchMapTileType { const packedByte = this.mapData[4 + y * this.width + x]; const isLand = packedByte & 0b10000000; - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const shoreline = !!(packedByte & 0b01000000); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const ocean = !!(packedByte & 0b00100000); const magnitude = packedByte & 0b00011111; if (isLand) { return SearchMapTileType.Land; } if (magnitude < 10) { return SearchMapTileType.Shore; } return SearchMapTileType.Water; }If the bits are reserved for future use, a comment explaining the bit layout would be more appropriate than unused variable declarations.
tests/core/executions/TradeShipExecution.test.ts (1)
26-26: Consider keeping argument validation in the mock.The simplified no-arg mock works but won't catch if production code passes incorrect arguments to
buildUnit. Consider:- buildUnit: jest.fn(() => tradeShip), + buildUnit: jest.fn((_type, _spawn, _opts) => tradeShip),This preserves argument count validation while still marking them unused. That said, the current approach is valid if argument validation isn't a priority for these tests.
src/client/graphics/layers/TerritoryLayer.ts (2)
451-454: Consider removing unused variable.The
isHighlightedvariable is declared but never used. If this is for a planned feature, consider removing it until needed to reduce code noise.Apply this diff to remove the unused variable:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const isHighlighted = - this.highlightedTerritory && - this.highlightedTerritory.id() === owner.id(); const myPlayer = this.game.myPlayer();
458-459: Consider removing unused variable.The
playerIsFocusedvariable is computed but never used in the border rendering logic. If this is preparatory work for a future feature, consider removing it until needed.Apply this diff to remove the unused variable:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const playerIsFocused = owner && this.game.focusedPlayer() === owner; if (myPlayer) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (76)
eslint.config.js(2 hunks)src/client/TerritoryPatternsModal.ts(1 hunks)src/client/Transport.ts(1 hunks)src/client/components/Difficulties.ts(1 hunks)src/client/components/NewsButton.ts(1 hunks)src/client/graphics/fx/SpriteFx.ts(1 hunks)src/client/graphics/layers/ChatIntegration.ts(1 hunks)src/client/graphics/layers/ChatModal.ts(1 hunks)src/client/graphics/layers/ControlPanel.ts(1 hunks)src/client/graphics/layers/EmojiTable.ts(1 hunks)src/client/graphics/layers/EventsDisplay.ts(2 hunks)src/client/graphics/layers/FPSDisplay.ts(1 hunks)src/client/graphics/layers/Leaderboard.ts(1 hunks)src/client/graphics/layers/PlayerInfoOverlay.ts(1 hunks)src/client/graphics/layers/PlayerPanel.ts(2 hunks)src/client/graphics/layers/RadialMenu.ts(3 hunks)src/client/graphics/layers/RadialMenuElements.ts(1 hunks)src/client/graphics/layers/RailroadLayer.ts(1 hunks)src/client/graphics/layers/StructureLayer.ts(1 hunks)src/client/graphics/layers/TeamStats.ts(1 hunks)src/client/graphics/layers/TerritoryLayer.ts(2 hunks)src/client/graphics/layers/WinModal.ts(1 hunks)src/client/jwt.ts(2 hunks)src/core/Schemas.ts(0 hunks)src/core/WorkerSchemas.ts(1 hunks)src/core/configuration/DefaultConfig.ts(2 hunks)src/core/configuration/DevConfig.ts(1 hunks)src/core/execution/AttackExecution.ts(2 hunks)src/core/execution/BoatRetreatExecution.ts(1 hunks)src/core/execution/CityExecution.ts(1 hunks)src/core/execution/ConstructionExecution.ts(2 hunks)src/core/execution/DefensePostExecution.ts(2 hunks)src/core/execution/DeleteUnitExecution.ts(2 hunks)src/core/execution/DonateGoldExecution.ts(2 hunks)src/core/execution/DonateTroopExecution.ts(2 hunks)src/core/execution/EmojiExecution.ts(2 hunks)src/core/execution/FactoryExecution.ts(1 hunks)src/core/execution/MIRVExecution.ts(2 hunks)src/core/execution/MarkDisconnectedExecution.ts(1 hunks)src/core/execution/MissileSiloExecution.ts(1 hunks)src/core/execution/MoveWarshipExecution.ts(2 hunks)src/core/execution/NoOpExecution.ts(1 hunks)src/core/execution/NukeExecution.ts(2 hunks)src/core/execution/PortExecution.ts(1 hunks)src/core/execution/QuickChatExecution.ts(2 hunks)src/core/execution/RailroadExecution.ts(2 hunks)src/core/execution/RetreatExecution.ts(1 hunks)src/core/execution/SAMLauncherExecution.ts(1 hunks)src/core/execution/SAMMissileExecution.ts(1 hunks)src/core/execution/ShellExecution.ts(1 hunks)src/core/execution/SpawnExecution.ts(1 hunks)src/core/execution/TargetPlayerExecution.ts(2 hunks)src/core/execution/TradeShipExecution.ts(1 hunks)src/core/execution/TrainExecution.ts(2 hunks)src/core/execution/TrainStationExecution.ts(1 hunks)src/core/execution/UpgradeStructureExecution.ts(2 hunks)src/core/execution/WarshipExecution.ts(2 hunks)src/core/execution/WinCheckExecution.ts(1 hunks)src/core/execution/alliance/AllianceExtensionExecution.ts(2 hunks)src/core/execution/alliance/AllianceRequestExecution.ts(2 hunks)src/core/execution/alliance/AllianceRequestReplyExecution.ts(2 hunks)src/core/execution/alliance/BreakAllianceExecution.ts(2 hunks)src/core/game/GameView.ts(2 hunks)src/core/game/StatsImpl.ts(2 hunks)src/core/game/TerrainSearchMap.ts(1 hunks)src/core/game/TrainStation.ts(2 hunks)src/server/GameServer.ts(1 hunks)src/server/Master.ts(1 hunks)src/server/Privilege.ts(2 hunks)src/server/Worker.ts(1 hunks)src/server/jwt.ts(3 hunks)tests/ShellRandom.test.ts(1 hunks)tests/core/executions/SAMLauncherExecution.test.ts(1 hunks)tests/core/executions/TradeShipExecution.test.ts(1 hunks)tests/util/TestConfig.ts(3 hunks)tests/util/TestServerConfig.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/core/Schemas.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/core/game/StatsImpl.tssrc/core/execution/BoatRetreatExecution.ts
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
PR: openfrontio/OpenFrontIO#1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
tests/core/executions/SAMLauncherExecution.test.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
PR: openfrontio/OpenFrontIO#1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
Applied to files:
src/client/graphics/layers/TerritoryLayer.ts
🧬 Code graph analysis (44)
src/core/execution/SAMMissileExecution.ts (3)
src/core/game/Game.ts (1)
Game(639-723)src/core/pathfinding/PathFinding.ts (1)
AirPathFinder(69-103)src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
src/core/execution/DonateGoldExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/RadialMenu.ts (1)
src/client/InputHandler.ts (1)
CloseViewEvent(71-71)
src/core/execution/SAMLauncherExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/RailroadExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/PlayerPanel.ts (1)
src/client/InputHandler.ts (1)
CloseViewEvent(71-71)
src/core/execution/TradeShipExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/TrainStationExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/ChatModal.ts (1)
src/client/InputHandler.ts (1)
CloseViewEvent(71-71)
src/core/execution/AttackExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
tests/util/TestServerConfig.ts (1)
src/core/Schemas.ts (1)
GameID(24-24)
src/core/execution/RetreatExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/RailroadLayer.ts (2)
src/core/game/GameView.ts (1)
x(648-650)src/core/game/GameImpl.ts (1)
x(780-782)
src/core/execution/MIRVExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/ConstructionExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/game/StatsImpl.ts (2)
src/core/game/Game.ts (2)
Player(503-637)TerraNullius(490-495)src/core/StatsSchemas.ts (3)
BOAT_INDEX_DESTROY(69-69)BOAT_INDEX_SENT(66-66)BOAT_INDEX_ARRIVE(67-67)
src/core/execution/QuickChatExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/EmojiTable.ts (1)
src/client/InputHandler.ts (1)
CloseViewEvent(71-71)
src/core/execution/CityExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/alliance/AllianceExtensionExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/TerritoryPatternsModal.ts (1)
src/core/CosmeticSchemas.ts (1)
Pattern(7-7)
src/core/execution/alliance/BreakAllianceExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/BoatRetreatExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/TrainExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/WinCheckExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
tests/util/TestConfig.ts (2)
src/core/game/Game.ts (3)
Game(639-723)Player(503-637)TerraNullius(490-495)src/core/game/GameMap.ts (1)
TileRef(3-3)
src/core/execution/MoveWarshipExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/WarshipExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/WinModal.ts (1)
src/core/CosmeticSchemas.ts (1)
Pattern(7-7)
src/client/graphics/layers/ChatIntegration.ts (1)
src/client/graphics/layers/RadialMenuElements.ts (1)
MenuElementParams(29-42)
src/core/execution/SpawnExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/client/graphics/layers/TerritoryLayer.ts (2)
src/client/InputHandler.ts (1)
DragEvent(60-65)src/core/game/GameView.ts (4)
owner(105-107)owner(613-615)myPlayer(569-571)tile(102-104)
src/core/execution/DeleteUnitExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/server/Master.ts (2)
src/core/configuration/DefaultConfig.ts (1)
workerPath(207-209)tests/util/TestServerConfig.ts (1)
workerPath(67-69)
src/core/execution/alliance/AllianceRequestReplyExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/PortExecution.ts (2)
src/core/game/Game.ts (1)
Game(639-723)src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
src/core/execution/MarkDisconnectedExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/NoOpExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/ShellExecution.ts (3)
src/core/game/Game.ts (1)
Game(639-723)src/core/pathfinding/PathFinding.ts (1)
AirPathFinder(69-103)src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
src/core/game/TrainStation.ts (2)
src/core/game/Game.ts (1)
Game(639-723)src/core/execution/TrainExecution.ts (1)
TrainExecution(14-242)
src/core/execution/EmojiExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/execution/MissileSiloExecution.ts (1)
src/core/game/Game.ts (1)
Game(639-723)
src/core/configuration/DefaultConfig.ts (1)
src/core/game/Game.ts (2)
Player(503-637)TerraNullius(490-495)
src/core/game/GameView.ts (1)
src/core/game/Game.ts (1)
PlayerID(14-14)
🔇 Additional comments (95)
src/client/Transport.ts (1)
405-405: Good unused parameter handling.Prefixing the unused
CloseEventargument with_keeps the signature clear while satisfying the new lint rule. Nicely done.src/server/Master.ts (1)
302-302: LGTM! Correct use of underscore prefix for unused parameter.Renaming
reqto_reqis the idiomatic way to handle unused parameters in TypeScript while maintaining the Express route handler signature.src/client/graphics/layers/EmojiTable.ts (1)
51-55: Rename looks solidThanks for switching to
_e. No behavior change and lint stays happy.src/core/execution/CityExecution.ts (1)
15-20: Underscore rename is fineThe
_ticksrename keeps intent clear and avoids the lint warning without touching logic.src/client/graphics/layers/PlayerPanel.ts (1)
170-182: Close handler rename approvedBoth listeners now show the argument is unused. Works cleanly with the rule.
src/core/execution/NukeExecution.ts (1)
35-41: Good underscore tweak
_tickssignals the arg is unused, nothing else changed. All good.Also applies to: 104-183
src/core/game/GameView.ts (1)
432-433: No issues with the renameAdding
_keeps the interface tidy and still returns the same value.Also applies to: 606-608
src/client/TerritoryPatternsModal.ts (1)
99-99: LGTM!The underscore prefix correctly signals that the pattern parameter is unused in this callback. This aligns with the PR's lint rule enforcement.
src/core/execution/EmojiExecution.ts (1)
22-22: LGTM!Renaming unused
ticksparameters to_tickscorrectly signals they're intentionally unused while maintaining the interface contract. This pattern is consistent across all execution classes in the PR.Also applies to: 35-35
src/core/WorkerSchemas.ts (1)
8-8: LGTM!The underscore prefix appropriately marks the transform parameter as unused while preserving the schema's behavior of returning
undefined.src/client/graphics/layers/RadialMenuElements.ts (1)
1-1: File-level disable is pragmatic for this menu DSL.This file contains many menu element configurations where parameters are intentionally unused in certain callbacks. The file-level ESLint disable is a reasonable choice here rather than renaming dozens of individual parameters across menu definitions.
src/core/execution/ShellExecution.ts (1)
21-21: LGTM!Renaming unused
ticksparameters to_ticksfollows the established pattern across execution classes while maintaining interface compliance.Also applies to: 27-27
src/server/Worker.ts (1)
513-513: LGTM!The underscore prefix correctly signals that the
nextfunction is unused in this error handler while maintaining the required Express middleware signature (4 parameters).src/core/execution/FactoryExecution.ts (1)
14-14: LGTM!Renaming unused
ticksparameters to_ticksmaintains interface compliance while clearly signaling the parameters are intentionally unused.Also applies to: 18-18
src/core/execution/SAMLauncherExecution.ts (1)
184-186: LGTM!The parameter rename from
ticksto_tickscorrectly signals that this parameter is intentionally unused in theinitmethod. This follows the new ESLint rule convention.src/core/execution/DefensePostExecution.ts (2)
20-22: LGTM!The parameter rename from
ticksto_ticksin theinitmethod correctly signals intentional non-use.
47-113: LGTM!The parameter rename from
ticksto_ticksin thetickmethod is correct. The parameter is unused because the targeting logic for defense posts is commented out (lines 70-112).tests/core/executions/SAMLauncherExecution.test.ts (1)
86-93: LGTM!Removing the unused
const nuke =binding is the right approach here since the test doesn't reference it. The unit is still created and the test behavior is unchanged.eslint.config.js (2)
40-46: LGTM!Switching the disabled rule from
@typescript-eslint/no-unused-varstono-unused-varsis correct, as you're now enforcing the TypeScript-specific rule below.
48-62: LGTM!The new
@typescript-eslint/no-unused-varsrule configuration is well-designed. The patterns (argsIgnorePattern,varsIgnorePattern,caughtErrorsIgnorePattern) correctly allow underscore-prefixed identifiers to be unused, which aligns with the changes throughout this PR.src/client/graphics/layers/EventsDisplay.ts (2)
463-468: LGTM!The parameter rename from
gameto_gamecorrectly signals that the parameter is unused in this callback. The logic only checks the alliance relationship betweenrequestorandrecipient.
1042-1112: LGTM!The parameter rename from
indexto_indexin the map callback correctly signals that the index is unused. The callback only operates on theeventobject.src/server/GameServer.ts (1)
495-517: LGTM!The catch binding rename from
eto_eis correct. The inner catch block doesn't need the error details since it's just falling back toString(error)for the outer error object.src/core/execution/WinCheckExecution.ts (1)
22-24: LGTM!The parameter rename from
ticksto_tickscorrectly signals intentional non-use in theinitmethod.src/core/execution/NoOpExecution.ts (1)
10-11: LGTM!The underscore-prefixed parameter names correctly signal that these parameters are intentionally unused in this no-op implementation.
src/core/execution/TrainStationExecution.ts (1)
25-25: LGTM!The
_ticksparameter is unused in theinitmethod body. The naming correctly signals this.src/core/execution/UpgradeStructureExecution.ts (2)
12-12: LGTM!Both
_mgand_ticksare unused in theinitmethod. The underscore prefix correctly signals this.
32-32: LGTM!The
_ticksparameter is unused in thetickmethod. The naming is correct.src/client/graphics/layers/FPSDisplay.ts (1)
162-162: LGTM!The
_frameDurationparameter is unused in the method body (the FPS calculation usesperformance.now()instead). The underscore prefix correctly signals this.src/core/execution/MoveWarshipExecution.ts (2)
11-11: LGTM!The
_ticksparameter is unused in theinitmethod. The underscore prefix correctly signals this.
31-31: LGTM!The
_ticksparameter is unused in the emptytickmethod. The naming is correct.src/core/execution/SAMMissileExecution.ts (2)
29-29: LGTM!The
_ticksparameter is unused in theinitmethod (the method callsmg.ticks()instead). The underscore prefix correctly signals this.
35-35: LGTM!The
_ticksparameter is unused in thetickmethod. The naming is correct.src/core/game/StatsImpl.ts (5)
179-179: LGTM!The
_targetparameter is unused in the current method body. The underscore prefix signals this while preserving the parameter for potential future use.Based on learnings.
194-194: LGTM!The
_targetparameter is unused in the current method body. The underscore prefix signals this while preserving the parameter for potential future use.Based on learnings.
198-204: LGTM!The
_targetand_troopsparameters are unused in the current method body. The underscore prefixes signal this while preserving the parameters for potential future use.Based on learnings.
206-212: LGTM!The
_targetand_troopsparameters are unused in the current method body. The underscore prefixes signal this while preserving the parameters for potential future use.Based on learnings.
214-220: LGTM!The
_targetand_troopsparameters are unused in the current method body. The underscore prefixes signal this while preserving the parameters for potential future use.Based on learnings.
src/core/execution/RailroadExecution.ts (2)
21-21: LGTM!The
_ticksparameter is unused in theinitmethod. The underscore prefix correctly signals this.
114-114: LGTM!The
_ticksparameter is unused in thetickmethod. The naming is correct.src/core/execution/PortExecution.ts (2)
19-23: LGTM! Clean unused parameter handling.The
ticksparameter was never referenced in the method body. Renaming to_tickscorrectly signals this is intentionally unused.
25-69: LGTM! Consistent with the init method.The
ticksparameter is unused here as well—the method usesthis.mg.ticks()directly (lines 21, 53). The underscore prefix is the right approach.src/core/execution/AttackExecution.ts (2)
46-164: LGTM! Unused parameter correctly marked.The
ticksparameter is never referenced in this method. The underscore prefix makes the intent clear.
203-290: LGTM! Consistent unused parameter handling.The
ticksparameter remains unused in the tick method as well. The rename is appropriate.src/core/execution/MIRVExecution.ts (2)
41-61: LGTM! Unused parameter marked clearly.The
ticksparameter is not used in the init method. The underscore prefix correctly signals this.
63-98: LGTM! Consistent approach.The
ticksparameter is unused in tick as well. The rename is appropriate.src/core/execution/TrainExecution.ts (2)
39-66: LGTM! Unused parameter correctly prefixed.The
ticksparameter is not referenced in this method. The underscore makes the intent explicit.
68-84: LGTM! Consistent handling.The
ticksparameter is also unused here. The rename is correct.src/core/execution/SpawnExecution.ts (2)
16-18: LGTM! Unused parameter clearly marked.The
ticksparameter is not used in this method. The underscore prefix is appropriate.
20-52: LGTM! Consistent pattern.The
ticksparameter is unused here as well. The rename is correct.src/core/execution/DonateTroopExecution.ts (2)
14-26: LGTM! Unused parameter handled correctly.The
ticksparameter is not referenced in this method. The underscore prefix is the right approach.
28-41: LGTM! Consistent with init.The
ticksparameter is also unused here. The rename is appropriate.src/core/execution/DeleteUnitExecution.ts (2)
16-74: LGTM! Unused parameter marked correctly.The
ticksparameter is never used in this method. The underscore prefix is appropriate.
76-76: LGTM! Empty method, unused parameter.This tick method is a no-op stub. The underscore prefix on the parameter is correct.
src/core/execution/ConstructionExecution.ts (2)
36-52: LGTM! Unused parameter correctly prefixed.The
ticksparameter is not referenced in this method. The underscore makes the intent clear.
54-99: LGTM! Consistent approach.The
ticksparameter is unused in tick as well. The rename is appropriate.src/core/execution/DonateGoldExecution.ts (1)
14-14: LGTM: Unused parameters correctly signaled.The underscore prefix clearly indicates these parameters satisfy the
Executioninterface but aren't used in this implementation. Clean and consistent with the PR's objectives.Also applies to: 25-25
src/core/execution/alliance/AllianceExtensionExecution.ts (1)
15-15: LGTM: Consistent unused parameter convention.The parameter renames align with the broader pattern across alliance execution classes. No behavioral changes.
Also applies to: 62-62
src/core/execution/alliance/AllianceRequestExecution.ts (1)
19-19: LGTM: Unused parameter correctly signaled.The
_ticksparameter is unused because the current tick is retrieved fromthis.mg.ticks()instead. This is correct and consistent.Also applies to: 48-48
src/core/execution/WarshipExecution.ts (1)
28-28: LGTM: Unused parameters properly indicated.Both method signatures correctly signal unused parameters while maintaining the
Executioninterface contract.Also applies to: 53-53
src/core/execution/alliance/AllianceRequestReplyExecution.ts (1)
13-13: LGTM: Consistent parameter naming.The underscore prefix appropriately signals unused parameters across both methods.
Also applies to: 24-24
src/core/execution/BoatRetreatExecution.ts (1)
10-10: LGTM: Unused parameters properly indicated.Both
_mgand_ticksare correctly prefixed to signal they're unused. These parameters are retained for interface compliance and potential future use. Based on learnings.Also applies to: 12-12
src/core/execution/TradeShipExecution.ts (1)
29-29: LGTM: Unused parameter correctly signaled.Line 29 correctly prefixes the unused
ticksparameter ininit. Note thattick(line 34) correctly keepstickswithout the underscore because it's actively used (line 47).src/core/execution/alliance/BreakAllianceExecution.ts (2)
13-13: LGTM!Parameter rename signals unused input, aligning with the lint rule.
25-25: LGTM!Parameter rename signals unused input, aligning with the lint rule.
src/core/execution/MarkDisconnectedExecution.ts (2)
9-9: LGTM!Parameter renames signal unused inputs, aligning with the lint rule.
13-13: LGTM!Parameter rename signals unused input, aligning with the lint rule.
src/core/execution/TargetPlayerExecution.ts (3)
13-13: LGTM!Parameter rename signals unused input, aligning with the lint rule.
23-23: LGTM!Parameter rename signals unused input, aligning with the lint rule.
23-29: No behavioral change intick: deactivation logic unchanged
Only the parameter was renamed to_ticks; thethis.active = falseline was unchanged and still executes every tick.Likely an incorrect or invalid review comment.
src/core/execution/RetreatExecution.ts (2)
15-15: LGTM!Parameter rename signals unused input, aligning with the lint rule.
20-20: LGTM!Parameter rename signals unused input, aligning with the lint rule.
src/core/game/TrainStation.ts (3)
57-61: LGTM!Parameter renames signal unused inputs in this stub handler, aligning with the lint rule.
169-171: LGTM!Parameter rename signals unused input, aligning with the lint rule.
177-179: LGTM!Parameter renames signal unused inputs, aligning with the lint rule.
src/core/configuration/DefaultConfig.ts (2)
754-756: LGTM!Parameter rename signals unused input, aligning with the lint rule.
770-776: LGTM!Parameter rename signals unused input, aligning with the lint rule.
src/server/Privilege.ts (3)
39-39: LGTM!Parameter rename signals unused input, aligning with the lint rule.
142-148: LGTM!Parameter renames signal unused inputs in this stub implementation, aligning with the lint rule.
150-155: LGTM!Parameter renames signal unused inputs in this stub implementation, aligning with the lint rule.
tests/util/TestServerConfig.ts (1)
58-75: LGTM!Parameter renames signal unused inputs in these test stub methods, aligning with the lint rule.
tests/util/TestConfig.ts (3)
24-24: LGTM!The parameter rename signals it's unused while maintaining the method signature for the override.
57-69: LGTM!All five parameters are correctly marked as unused. The method returns a hardcoded test fixture, so the parameters are intentionally ignored.
71-78: LGTM!Parameter renames correctly indicate these are unused in the test implementation.
src/client/graphics/layers/ControlPanel.ts (1)
109-111: LGTM!The parameter rename correctly signals the canvas context is unused. This Layer uses LitElement's HTML rendering instead of canvas rendering.
src/client/graphics/layers/WinModal.ts (1)
165-165: LGTM!The parameter rename indicates this is an intentional no-op handler. The pattern button requires an onSelect callback, but this modal only handles purchases.
src/client/graphics/layers/ChatModal.ts (1)
178-182: LGTM!The event parameter is correctly marked as unused. The handler only needs the event trigger, not any event data.
src/client/graphics/layers/PlayerInfoOverlay.ts (1)
145-147: LGTM!The parameter rename correctly indicates the canvas context is unused. This Layer uses LitElement rendering rather than canvas operations.
src/client/graphics/layers/TerritoryLayer.ts (2)
231-234: LGTM!The parameter rename correctly indicates the event data is unused. The handler body contains only a TODO and commented code for potential future optimization.
450-450: LGTM!The blank line improves readability by separating the owner extraction from subsequent logic.
src/client/graphics/layers/Leaderboard.ts (1)
159-159: LGTM!The parameter rename correctly signals the canvas context is unused. This Layer uses LitElement rendering instead of canvas operations.
src/client/graphics/layers/TeamStats.ts (1)
107-107: LGTM!The parameter rename correctly indicates the canvas context is unused. This Layer uses LitElement rendering rather than canvas operations.
src/client/graphics/layers/RadialMenu.ts (3)
111-111: LGTM!The parameter rename to
_ecorrectly signals that the event parameter is unused in the handler body. This follows the underscore convention for unused parameters and satisfies the ESLint rule.
158-158: LGTM!The parameter rename to
_eventcorrectly signals that the event parameter is unused in the handler body. This follows the underscore convention for unused parameters and satisfies the ESLint rule.
1009-1009: LGTM!The parameter rename to
_contextcorrectly signals that the canvas context parameter is unused in the implementation. SinceRadialMenuimplements theLayerinterface, the parameter is required for interface compliance even though this particular implementation renders to the DOM rather than the canvas.
| } | ||
|
|
||
| const data = await response.json(); | ||
| await response.json(); |
There was a problem hiding this comment.
Remove unused await response.json() call.
The result of response.json() is discarded. If you don't need the response body, remove this line. If you're consuming it intentionally to prevent resource leaks, add a comment explaining why.
Apply this diff to remove the unused call:
- await response.json();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await response.json(); |
🤖 Prompt for AI Agents
In src/server/Master.ts around line 289, remove the unused await response.json()
call (or if the body must be consumed to avoid resource leaks, replace it with a
clear inline comment explaining why the body is read and leave it). Specifically
delete the lone await response.json(); statement or add a comment like "//
consume body to free resources" before it if intentional.
| }); | ||
|
|
||
| const uniqueDamages = new Set(damages); | ||
| new Set(damages); |
There was a problem hiding this comment.
Remove dead code.
This line constructs a Set but doesn't store or use it. It serves no purpose and should be deleted entirely.
Apply this diff:
- new Set(damages);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| new Set(damages); |
🤖 Prompt for AI Agents
In tests/ShellRandom.test.ts around line 88, the statement "new Set(damages);"
is dead code — it creates a Set but neither assigns it nor uses it; remove this
line entirely so there is no unused object construction and the test code
remains clean.
|
Is there a way we can ignore unused parameters instead of _ prefix? |
Thanks for the question! "@typescript-eslint/no-unused-vars": [
"error",
{
args: "none",
argsIgnorePattern: "^_",
varsIgnorePattern: "^_",
caughtErrors: "none"
},
],With this setup, unused parameters don’t trigger errors even without the _ prefix, while other unused variables are still checked. For example: ✅ Unused parameters do not need _: function example(a, b) {
console.log(a);
// `b` is unused but no error
}✅ catch parameters are also ignored: try {
doSomething();
} catch (e) {
// not using `e` is fine
}❌ Regular unused variables still need _ to be ignored: const unused = 123; // ❌ error
const _ignored = 123; // ✅ ignoredIf this direction makes sense for the project, I can update the PR accordingly — just let me know your preference before I change anything. |
Description:
✅ Summary of Changes
This PR enables the ESLint rule
@typescript-eslint/no-unused-varsas requested in the issue and applies the necessary code adjustments across the project.🔧 What was done:
@typescript-eslint/no-unused-varsin the ESLint config._prefix where appropriate.eslint-disable-next-line) for specific cases where the variable or code block seemed important for context, readability, or future use.❓ Clarification
Some cases were handled with inline disable comments instead of removing the variable entirely, to avoid accidental breaking changes or loss of intent.
If a different approach is preferred (e.g., stricter removal or alternative handling), I’m happy to adjust the implementation accordingly — just let me know!
🙌 Next Steps
Please review and let me know if:
I’m available to make any follow-up improvements needed.
🎃 Hacktoberfest Note
I'm participating in Hacktoberfest, so if this PR is accepted, please add the label:
hacktoberfest-acceptedThank you!
#1784
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME