Add alliance request to main dial - #4974
Conversation
WalkthroughThe change calculates alliance-request cooldowns in the player model, exposes them through player interaction data, and renders cooldown styling, text, opacity, positioning, and icon updates in the radial menu. ChangesAlliance cooldown flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PlayerImpl
participant GameRunner
participant RadialMenuElements
participant RadialMenu
PlayerImpl->>GameRunner: calculate alliance request cooldown
GameRunner->>RadialMenuElements: provide cooldown in interaction data
RadialMenuElements->>RadialMenu: expose cooldown and styling data
RadialMenu->>RadialMenu: render cooldown state and refresh icons
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/hud/layers/RadialMenu.ts`:
- Around line 705-709: The cooldown icon positioning must apply to all aspect
ratios and update when cooldown state changes. In
src/client/hud/layers/RadialMenu.ts lines 705-709, move the image y-position
assignment before the square-icon aspect-ratio early return; in lines 1209-1215,
recompute the image y position whenever isAllianceCooldown changes so expiration
restores the centered position.
In `@src/core/game/PlayerImpl.ts`:
- Around line 739-785: Add deterministic core tests covering every zero-return
gate and cooldown boundary in PlayerImpl.allianceRequestCooldownRemaining,
including interval rounding and expiry; exercise the Player API through Game’s
playerActions path using a test implementation or fixture, and verify
GameRunner.playerActions forwards the calculated cooldown. Update
src/core/game/PlayerImpl.ts lines 739-785, src/core/game/Game.ts lines 671-671,
and src/core/GameRunner.ts lines 265-266 with the necessary test coverage; no
direct production change is required unless needed to make these tests possible.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b09eb2fb-95e4-432e-8cee-3960787304df
📒 Files selected for processing (5)
src/client/hud/layers/RadialMenu.tssrc/client/hud/layers/RadialMenuElements.tssrc/core/GameRunner.tssrc/core/game/Game.tssrc/core/game/PlayerImpl.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/client/hud/layers/RadialMenu.ts (2)
1258-1290: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winIgnore stale asynchronous icon renders.
refreshAllyExtendIcon()clears the group and starts a new asynchronous render when the cooldown or agreement state changes. An oldergetSvgAspectRatio()promise can resolve after the newer render and append stale handshake images.Track a per-content render generation and ignore callbacks from older renders.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/hud/layers/RadialMenu.ts` around lines 1258 - 1290, The asynchronous rendering flow in refreshAllyExtendIcon and renderAllyExtendIcon must ignore stale getSvgAspectRatio callbacks. Track a per-icon/content render generation, increment it whenever a new render starts, and have each async callback verify its generation is still current before appending handshake images or updating the group.
718-718: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the cooldown label through
translateText().The cooldown value is user-visible, but the code writes it directly with
.text(...). Use a translation key and pass the numeric cooldown as a parameter.As per coding guidelines:
**/*.{ts,tsx}requires all user-visible text to go throughtranslateText().Also applies to: 1227-1227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/hud/layers/RadialMenu.ts` at line 718, Update the cooldown labels at both visible `.text(String(cooldown))` call sites in the RadialMenu implementation to use translateText() with a translation key and the numeric cooldown as a parameter, ensuring the user-visible value follows the translation requirement.Source: Coding guidelines
♻️ Duplicate comments (1)
src/client/hud/layers/RadialMenu.ts (1)
693-710: 🎯 Functional Correctness | 🟡 MinorSynchronize cooldown layout during refresh.
The aspect-ratio early return is fixed, but
refresh()still changes only opacity and existing text. It does not recalculate the imageyposition or create.cooldown-textwhen the cooldown becomes active.A visible menu can keep the cooldown position after expiry or show no cooldown number after activation. Recompute the position and create or remove the label on every cooldown transition.
This is the same unresolved positioning issue reported in the previous review comment.
Also applies to: 1205-1229
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/hud/layers/RadialMenu.ts` around lines 693 - 710, Update RadialMenu.refresh() to fully synchronize cooldown state transitions, not just opacity and existing text. Recalculate each cooldown image’s y position when the cooldown activates or expires, and create the .cooldown-text label on activation or remove it when inactive. Preserve the existing width, height, x positioning, and text-update behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/GameRunner.test.ts`:
- Around line 127-133: Update the test around playerActions to create an
alliance request between player and other, reject that request before calling
runner.playerActions(), then assert allianceRequestCooldownRemaining is greater
than zero and equals player.allianceRequestCooldownRemaining(other). Keep the
existing interaction payload assertions.
---
Outside diff comments:
In `@src/client/hud/layers/RadialMenu.ts`:
- Around line 1258-1290: The asynchronous rendering flow in
refreshAllyExtendIcon and renderAllyExtendIcon must ignore stale
getSvgAspectRatio callbacks. Track a per-icon/content render generation,
increment it whenever a new render starts, and have each async callback verify
its generation is still current before appending handshake images or updating
the group.
- Line 718: Update the cooldown labels at both visible `.text(String(cooldown))`
call sites in the RadialMenu implementation to use translateText() with a
translation key and the numeric cooldown as a parameter, ensuring the
user-visible value follows the translation requirement.
---
Duplicate comments:
In `@src/client/hud/layers/RadialMenu.ts`:
- Around line 693-710: Update RadialMenu.refresh() to fully synchronize cooldown
state transitions, not just opacity and existing text. Recalculate each cooldown
image’s y position when the cooldown activates or expires, and create the
.cooldown-text label on activation or remove it when inactive. Preserve the
existing width, height, x positioning, and text-update behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4ee6578-84e5-4f4b-94b5-a76feda9b5c9
📒 Files selected for processing (3)
src/client/hud/layers/RadialMenu.tstests/PlayerImpl.test.tstests/core/GameRunner.test.ts
| const expected = player.allianceRequestCooldownRemaining(other); | ||
| const actions = runner.playerActions(player.id(), 50, 50); | ||
|
|
||
| expect(actions.interaction).toBeDefined(); | ||
| expect(actions.interaction?.allianceRequestCooldownRemaining).toBe( | ||
| expected, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test a non-zero cooldown value.
expected is zero because no alliance request was rejected. A payload that always sets allianceRequestCooldownRemaining to 0 will pass this test.
Create and reject a request before calling playerActions(). Assert that the forwarded value is greater than zero and equals the player value.
Proposed test update
const runner = new GameRunner(
game,
new Executor(game, gameID, undefined, []),
() => {},
);
+ const request = player.createAllianceRequest(other);
+ expect(request).not.toBeNull();
+ request!.reject();
+
const expected = player.allianceRequestCooldownRemaining(other);
+ expect(expected).toBeGreaterThan(0);
const actions = runner.playerActions(player.id(), 50, 50);📝 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.
| const expected = player.allianceRequestCooldownRemaining(other); | |
| const actions = runner.playerActions(player.id(), 50, 50); | |
| expect(actions.interaction).toBeDefined(); | |
| expect(actions.interaction?.allianceRequestCooldownRemaining).toBe( | |
| expected, | |
| ); | |
| const request = player.createAllianceRequest(other); | |
| expect(request).not.toBeNull(); | |
| request!.reject(); | |
| const expected = player.allianceRequestCooldownRemaining(other); | |
| expect(expected).toBeGreaterThan(0); | |
| const actions = runner.playerActions(player.id(), 50, 50); | |
| expect(actions.interaction).toBeDefined(); | |
| expect(actions.interaction?.allianceRequestCooldownRemaining).toBe( | |
| expected, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/core/GameRunner.test.ts` around lines 127 - 133, Update the test around
playerActions to create an alliance request between player and other, reject
that request before calling runner.playerActions(), then assert
allianceRequestCooldownRemaining is greater than zero and equals
player.allianceRequestCooldownRemaining(other). Keep the existing interaction
payload assertions.
1 participant
Resolves #4970
Description:
Addition of alliance cooldown onto the radial menu.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found: LDTigerboy