Meta: Feature/alliance cutoff#3884
Open
Zixer1 wants to merge 4 commits intoopenfrontio:mainfrom
Open
Conversation
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR implements a configurable alliance cutoff tick: warns players at -5 and -1 minutes, shows a client countdown and center overlay, blocks new alliance actions at the cutoff, expires existing alliances at the cutoff, and adds tests and config hooks. ChangesAlliance Cutoff Tick
Sequence Diagram(s)sequenceDiagram
participant Config
participant GameImpl
participant AllianceSubsystem
participant ClientEvents
Config->>GameImpl: alliancesCutoffTick()
GameImpl->>GameImpl: checkAlliancesCutoff() each tick
GameImpl->>AllianceSubsystem: expire alliances & reject pending (at cutoff)
GameImpl->>ClientEvents: broadcast warnings & final message
ClientEvents->>ClientEvents: EventsDisplay updates countdown/overlay
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/graphics/layers/EventsDisplay.ts`:
- Around line 834-851: Run Prettier on the file to fix formatting issues causing
CI failure; specifically reformat the CSS `@keyframes` block (named fadeInOut)
shown in the diff and the other similar blocks in the same file (the sections
around the ranges noted: 871-888 and 960-972), then add and commit the formatted
file so CI can pass. Ensure you use the project's configured Prettier settings
(e.g., npm/yarn prettier or editor integration) and do not alter the animation
semantics—only update whitespace/formatting.
- Around line 284-286: The early return when cutoff is reached skips cleanup of
stale renewal prompts; before returning from the block that computes cutoff (the
lines using cutoff, this.game.config().alliancesCutoffTick() and
this.game.ticks()), call this.removeAllianceRenewalEvents() and also prune/clear
the alliancesCheckedAt tracking (either by calling the existing pruning helper
or resetting the alliancesCheckedAt map/object) so “renew alliance” prompts are
removed when alliances are disabled.
In `@src/client/graphics/layers/NameLayer.ts`:
- Around line 174-177: The newly added multiline if in NameLayer (the block
using cutoff = this.config.alliancesCutoffTick() and the conditional checking
cutoff !== null && !this.alliancesDisabled && this.game.ticks() >= cutoff) is
not formatted to project Prettier rules; run the project's Prettier formatter
(or npm script) on src/client/graphics/layers/NameLayer.ts and commit the
resulting changes so the if-statement and surrounding lines match the
repository's formatting style, then ensure the file passes CI.
In `@src/client/graphics/PlayerIcons.ts`:
- Around line 108-112: The boolean merge for alliance cutoff is wrong: replace
the nullish coalescing used to compute alliancesOff with a logical OR so false
values are respected; specifically, change the expression that sets alliancesOff
(currently using alliancesDisabled ?? (game.config().disableAlliances() ||
pastCutoff)) to use alliancesDisabled || game.config().disableAlliances() ||
pastCutoff so that a false alliancesDisabled doesn’t short-circuit the
config/cutoff checks.
🪄 Autofix (Beta)
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
Run ID: aff6b6f7-7673-429e-98ab-23455699a8a7
📒 Files selected for processing (16)
resources/lang/en.jsonsrc/client/Utils.tssrc/client/graphics/PlayerIcons.tssrc/client/graphics/layers/EventsDisplay.tssrc/client/graphics/layers/NameLayer.tssrc/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/core/execution/TribeExecution.tssrc/core/execution/alliance/AllianceExtensionExecution.tssrc/core/execution/nation/NationAllianceBehavior.tssrc/core/game/Game.tssrc/core/game/GameImpl.tssrc/core/game/PlayerImpl.tstests/AlliancesCutoff.test.tstests/NationAllianceBehavior.test.tstests/util/TestConfig.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Some games can sometimes last for hours in a stalemate. I was thinking a simple fix could be that at a certain point, like 45min, all alliances are auto disabled. It wont explode the map into chaos since players will still want to play smart and not mindlessly attack, but it will enable piracy and pettier border conflicts, which can evolve into a full push, speeding up the end game stalemates which sometimes are way too excessive.
All the UI additions:
589117387-ba2dc68c-0a78-4b38-bdac-22d2bf49e859.mov
Screen.Recording.2026-05-07.at.3.17.52.PM.mov
Screen.Recording.2026-05-07.at.3.16.37.PM.mov
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
zixer._