Added notifcation when a player wants to renew

[RjManhas]

Description:

Describe the PR.

Added a new chat message from the server once player wants to renew the alliance, to the other player.

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

notifxy (1379678982676676639)

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Walkthrough

Adds a localization key and updates alliance-extension execution to notify the other player with a RENEW_ALLIANCE "wants_to_renew_alliance" message when the first player newly agrees to renew an alliance (transition from 0 → 1 agreement).

Changes

Cohort / File(s)	Summary
Localization resources/lang/en.json	Added events_display.wants_to_renew_alliance = "{name} wants to renew your alliance".
Alliance Extension Logic src/core/execution/alliance/AllianceExtensionExecution.ts	Record pre-state wasOnlyOneAgreed; when transitioning from 0 → 1 agreement, send RENEW_ALLIANCE message to the other player using the new localization key; existing both-agree path unchanged.
Tests tests/AllianceExtensionExecution.test.ts	Added test verifying a single renewal request sends a RENEW_ALLIANCE display message to the other player and does not duplicate on subsequent requests; updated imports to include MessageType.

Sequence Diagram

sequenceDiagram
    participant P1 as Player 1
    participant S as System
    participant P2 as Player 2

    P1->>S: Request alliance extension
    activate S
    Note over S: Read pre-state (wasOnlyOneAgreed?)

    alt Transition 0 → 1 agreement
        S->>P2: Send RENEW_ALLIANCE ("wants_to_renew_alliance")
        Note over P2: Shows "{name} wants to renew your alliance"
    else No new single-agreement transition
        Note over S: Do not re-send renew prompt
    end

    alt Both players agree
        S->>S: Apply alliance extension
        Note over S: Alliance extended
    end
    deactivate S

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify correctness of the wasOnlyOneAgreed detection and that the message is only emitted on the 0 → 1 transition.
• Confirm message parameters (name) are passed into translation/display.
• Check tests for flakiness and that no duplicate messages occur on repeated requests.

Possibly related PRs

• Fix oversight: non-human player never responds to alliance renewal request #1536 — Related changes to alliance-extension logic and agreement/notification flow.
• Fix: display actual Alliance extension message instead of translation key #1532 — Touches translation/parameter usage for alliance-renewal messages in the same flow.
• allow trains & railroads between non-allies #1719 — Overlaps modifications to AllianceExtensionExecution.ts and extension conditions.

Suggested labels

Feature - New, Translation, Feature - Test

Suggested reviewers

• evanpelle
• scottanderson

Poem

🤝 One nod sent across the lane,

A prompt appears, a friendly claim,
"{name} wants to renew" on view,
One ask — another may say true,
Small code, big nudge toward clan and name.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding a notification when a player requests alliance renewal.
Description check	✅ Passed	The description clearly relates to the changeset by explaining the new server chat message feature and documenting completion of all required checklist items.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 581e065 and d7be807.

📒 Files selected for processing (1)

• tests/AllianceExtensionExecution.test.ts (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.

🧬 Code graph analysis (1)

tests/AllianceExtensionExecution.test.ts (3)

src/core/execution/alliance/AllianceRequestExecution.ts (1)

• AllianceRequestExecution (9-72)

src/core/execution/alliance/AllianceRequestReplyExecution.ts (1)

• AllianceRequestReplyExecution (3-56)

src/core/execution/alliance/AllianceExtensionExecution.ts (1)

• AllianceExtensionExecution (9-86)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: 🔬 Test

🔇 Additional comments (3)

tests/AllianceExtensionExecution.test.ts (3)

4-4: LGTM! Import is necessary and correct.

The MessageType import is needed for verifying the message type in the new test case.

---

123-157: Well-structured test verifying the renewal notification.

The test correctly:

• Establishes an alliance between two players
• Spies on displayMessage to capture the notification
• Verifies the message is sent to the other player with correct parameters
• Checks the localization key, message type, recipient, and requester's name

The assertions are precise and verify the exact behavior of the new feature.

---

159-167: Excellent coverage of the deduplication behavior.

The test verifies that subsequent renewal requests from the same player don't trigger duplicate notifications. This correctly tests the wasOnlyOneAgreed logic in the implementation that prevents repeat messages.

The spy restoration at the end is good practice.

---

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]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/execution/alliance/AllianceExtensionExecution.ts (1)

1-86: Fix formatting issue before merge.

The pipeline indicates code style issues that need to be resolved.

Run the following command to fix the formatting:

npx prettier --write src/core/execution/alliance/AllianceExtensionExecution.ts

🧹 Nitpick comments (1)

src/core/execution/alliance/AllianceExtensionExecution.ts (1)

1-86: Consider adding tests for the notification logic.

The contributor checklist mentions adding relevant tests. Consider adding test cases for:

• Notification sent when first player requests renewal
• No duplicate notification when same player requests again
• Both players agreeing triggers renewal message (existing behavior)

Would you like me to help generate test cases for this functionality?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe81cb and 19c8d46.

📒 Files selected for processing (2)

• resources/lang/en.json (1 hunks)
• src/core/execution/alliance/AllianceExtensionExecution.ts (2 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.

📚 Learning: 2025-06-02T14:27:37.609Z

Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

• resources/lang/en.json

🪛 GitHub Actions: 🧪 CI

src/core/execution/alliance/AllianceExtensionExecution.ts

[warning] 1-1: Code style issues found. Run 'npx prettier --write' to fix formatting.

🔇 Additional comments (3)

resources/lang/en.json (1)

579-579: LGTM! Clean localization string.

The new message follows the existing patterns and is correctly placed in the events_display section.

src/core/execution/alliance/AllianceExtensionExecution.ts (2)

39-40: Good approach to prevent duplicate notifications.

Capturing the state before adding the extension request ensures the notification is sent only on the first request, not on subsequent duplicate requests from the same player.

---

62-72: MessageType.RENEW_ALLIANCE is defined and properly integrated.

Verification confirms MessageType.RENEW_ALLIANCE exists in the codebase (defined at src/core/game/Game.ts:818, categorized at line 850, and used consistently across multiple files). The logic correctly identifies the transition from zero to one agreement and sends the notification to the other player. No issues found.

[RjManhas]

the issue for this pr #2392

[evanpelle]

thanks!

[evanpelle]

Thanks!