Conversation
|
|
WalkthroughTransportShips gain a maximum health property and warship shell attack rates increase from 20 to 40. Shell execution now applies fixed troop losses to transport targets instead of standard HP damage, and warship firing logic adjusts attack rates based on target type, with special handling for transport ships to prevent immediate one-shot elimination. Changes
Sequence DiagramsequenceDiagram
actor Warship
participant WarshipExecution
participant ShellExecution
participant TransportShip
Warship->>WarshipExecution: shootTarget()
WarshipExecution->>WarshipExecution: Compute effectiveAttackRate<br/>(reduced for TransportShip)
alt Firing condition met
WarshipExecution->>ShellExecution: fireShell(target)
ShellExecution->>ShellExecution: Shell path completes
ShellExecution->>TransportShip: Check if target is TransportShip
alt Is TransportShip
ShellExecution->>TransportShip: applyFixedTroopLossOnTransport(80000)
TransportShip->>TransportShip: Reduce troops, check if depleted
alt Troops depleted
TransportShip-->>ShellExecution: Signal deletion
ShellExecution->>ShellExecution: Mark shell reached & remove
else Troops remain
ShellExecution->>ShellExecution: Continue to HP damage
end
else Not TransportShip
ShellExecution->>TransportShip: Apply standard HP damage
ShellExecution->>ShellExecution: Mark shell reached & remove
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
🧹 Nitpick comments (2)
src/core/execution/ShellExecution.ts (2)
85-88: Edge case: transport with zero troops is not deleted.When
currentTroops <= 0, the method returnstrue(shell handled) but the transport is not deleted. If a transport can exist with zero troops, it would remain alive after this hit.If this is intentional (e.g., transport already marked for deletion elsewhere), this is fine. Otherwise, consider deleting the transport here too:
♻️ Optional: delete transport when already at zero troops
const currentTroops = this.target.troops(); if (currentTroops <= 0) { + this.target.delete(true, this._owner); return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/execution/ShellExecution.ts` around lines 85 - 88, The early-return when currentTroops <= 0 in ShellExecution (using this.target.troops() stored in currentTroops) leaves a transport object alive; modify the branch so that when currentTroops <= 0 you also remove/delete the transport (call the appropriate deletion method on the target/transport manager or mark it for removal) before returning true; locate the code in the ShellExecution method that reads currentTroops and ensure you invoke the transport deletion API (e.g., this.target.destroy(), this.target.delete(), or the project’s transport removal helper) then return true.
91-91: Move shell troop loss constant to config for easier balance adjustments.The code has
80000as the fixed troop loss per shell hit on transport ships. While the suggestion to move this constant toDefaultConfig(like other balance values such aswarshipShellAttackRate()) is reasonable for maintainability, note that balance tuning constants change frequently and do not require code changes if properly configured—this is an optional refactoring for consistency with the existing pattern, not a critical fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/execution/ShellExecution.ts` at line 91, Move the hardcoded fixedLossPerHit = 80000 into the configuration system: add a new getter (e.g., transportShellFixedLoss()) to DefaultConfig alongside existing getters like warshipShellAttackRate(), update ShellExecution to read DefaultConfig.transportShellFixedLoss() instead of the literal fixedLossPerHit, and ensure any tests or call sites referencing fixedLossPerHit are updated to use the config getter so balance tuning can be changed without code edits.
🤖 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/execution/ShellExecution.ts`:
- Around line 85-88: The early-return when currentTroops <= 0 in ShellExecution
(using this.target.troops() stored in currentTroops) leaves a transport object
alive; modify the branch so that when currentTroops <= 0 you also remove/delete
the transport (call the appropriate deletion method on the target/transport
manager or mark it for removal) before returning true; locate the code in the
ShellExecution method that reads currentTroops and ensure you invoke the
transport deletion API (e.g., this.target.destroy(), this.target.delete(), or
the project’s transport removal helper) then return true.
- Line 91: Move the hardcoded fixedLossPerHit = 80000 into the configuration
system: add a new getter (e.g., transportShellFixedLoss()) to DefaultConfig
alongside existing getters like warshipShellAttackRate(), update ShellExecution
to read DefaultConfig.transportShellFixedLoss() instead of the literal
fixedLossPerHit, and ensure any tests or call sites referencing fixedLossPerHit
are updated to use the config getter so balance tuning can be changed without
code edits.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/configuration/DefaultConfig.tssrc/core/execution/ShellExecution.tssrc/core/execution/WarshipExecution.ts
2 participants
Description:
This PR rebalances naval interactions to make transport-vs-warship engagements less binary and more readable during fights, while keeping the existing combat system intact.
What changed
1. Warship firing cadence adjusted by target type
◦ Warship shell cadence is now context-aware:
▪ Faster cadence against transport ships.
2. Transport hits now apply fixed troop loss
◦ When a shell reaches a transport, damage is resolved primarily through troop loss, using a -8000 troops fixed per-hit amount.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
HulKiora