visibleAt

[ryanbarlow97]

Description:

needs prereq of https://github.com/openfrontio/infra/pull/272

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:

w.o.n

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d70d0154-5982-4e57-977d-5654566a0bf7

📥 Commits

Reviewing files that changed from the base of the PR and between e4faee4 and 033692b.

📒 Files selected for processing (2)

• src/client/ClientGameRunner.ts
• src/core/Util.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/client/ClientGameRunner.ts
• src/core/Util.ts

---

Walkthrough

Added an optional visibleAt timestamp and propagated it through schema, server, utility, and client code: schema accepts visibleAt, GameServer sets/passes it at start, createPartialGameRecord records and uses it to compute lobby fill time, and the client includes it when saving game records.

Changes

Cohort / File(s)	Summary
Schema Updates src/core/Schemas.ts	Added optional visibleAt: z.number().optional() to GameStartInfoSchema, updating the inferred GameStartInfo type with visibleAt?: number.
Utility Function Enhancement src/core/Util.ts	createPartialGameRecord(..., lobbyCreatedAt?, visibleAt?) signature updated to accept visibleAt; it sets PartialGameRecord.info.visibleAt and computes lobbyFillTime as start - (visibleAt ?? actualLobbyCreatedAt) clamped with Math.max(0, ...).
Server Implementation src/server/GameServer.ts	Added optional visibleAt?: number; initialized when constructed with startsAt and lazily in setStartsAt. Include visibleAt in GameStartInfo validation payload and pass it to createPartialGameRecord when finalizing/archiving records.
Client Integration src/client/ClientGameRunner.ts	saveGame now supplies visibleAt from this.lobby.gameStartInfo.visibleAt to createPartialGameRecord/game record creation.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant Server as GameServer
  participant Validator as SchemaValidator
  participant Util as Util/createPartialGameRecord
  participant Store as Storage

  rect rgba(200,200,255,0.5)
    Client->>Server: request/set startsAt
    Server-->>Server: this.visibleAt ??= Date.now()
  end

  Server->>Validator: validate GameStartInfo (gameID, lobbyCreatedAt, config, players, visibleAt)
  Validator-->>Server: valid
  Server->>Util: createPartialGameRecord(start, lobbyCreatedAt, visibleAt, winner?)
  Util-->>Store: write partial/final GameRecord (includes info.visibleAt)
  Store-->>Client: ack/save confirmation

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A small timestamp slips through the gate,
It notes when lobbies first await.
Schema, server, util, and save,
Carry that moment to the archive cave.
Tiny mark — the timeline's mate 🎮

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'visibleAt' is vague and does not clearly convey what the pull request changes or accomplishes.	Consider using a more descriptive title like 'Add visibleAt timestamp to game start info' that explains the purpose and scope of the change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description mentions a prerequisite and includes a completed checklist, but lacks technical details about the actual changes being made.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.}

[ryanbarlow97]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

🧹 Nitpick comments (1)

src/server/GameServer.ts (1)

562-566: Move field declaration to the top of the class with other private fields.

The visibleAt field is declared in the middle of the class (between methods), while all other private fields are grouped together at lines 42-88. Moving this field up keeps the class structure consistent and easier to scan.

The ??= operator in setStartsAt is correct — it ensures only the first call sets the visibility timestamp.

♻️ Suggested change

Move the field declaration to the fields section (around line 59):

   private hasReachedMaxPlayerCount: boolean = false;
+  private visibleAt?: number;

   private endTurnIntervalID: ReturnType<typeof setInterval> | undefined;

Then remove the declaration from line 562-563.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/GameServer.ts` around lines 562 - 566, The private field visibleAt
is declared mid-class; move its declaration up into the other private fields
block alongside the other fields at the top of the GameServer class (so it is
declared once with the other private fields) and delete the duplicate
declaration at its current location; keep the logic in the setStartsAt method
(which uses visibleAt ??= Date.now()) unchanged so only the field declaration is
relocated.

🤖 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/server/GameServer.ts`:
- Around line 562-566: The private field visibleAt is declared mid-class; move
its declaration up into the other private fields block alongside the other
fields at the top of the GameServer class (so it is declared once with the other
private fields) and delete the duplicate declaration at its current location;
keep the logic in the setStartsAt method (which uses visibleAt ??= Date.now())
unchanged so only the field declaration is relocated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41a595cf-f167-4183-8534-6d699b973fdf

📥 Commits

Reviewing files that changed from the base of the PR and between bd3bf5f and 3749b53.

📒 Files selected for processing (2)

• src/core/Util.ts
• src/server/GameServer.ts