renderer: day-state plate selection (#1291)#1298
Conversation
Adds an additive, default-off day/night plate-swap rule to paint_combat_v1.cs's existing plate-resolution block. When a _time_of_day.txt sentinel on the box says "day" and a naming-convention sibling <room>_day_v1.png exists (room = the resolved plate's prefix up to the first underscore), the renderer prefers the day plate. No sentinel, sentinel != "day", or a missing day plate all fall through unchanged to today's night/current behavior. The engine's campaign.time_of_day is not yet surfaced on /combat-surface, so this wires via a box-side sentinel file (mirroring the existing _active_combat.txt / _location_plates.json pattern) rather than a new engine field — the minimal wiring for the generation-is-done, wiring-is-open half of #1291.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1298 - renderer: day-state plate selection (#1291) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1298 Review URL: #1298 (review) |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #1298 - renderer: day-state plate selection (#1291)
Head: d2e8a76a1f0d7acd6e97d754ad1a96a8051dd914 into main. Review event: COMMENT.
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
extensions/renderers/unity/scripts/paint_combat_v1.cs |
modified | +19/-0 | Unity/gameplay state | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Unity scene/prefab: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - Unity asset/script/project files changed. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1291.
Suggested labels: unity.
Suggested reviewers: none from current metadata.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
| if(_tod=="day"){ | ||
| var _us=PLATE.IndexOf('_'); | ||
| var _room=_us>0?PLATE.Substring(0,_us):PLATE.Replace(".png",""); | ||
| var _dayName=_room+"_day_v1.png"; |
There was a problem hiding this comment.
P3: Day-plate name hardcodes _v1, diverges from curated _v2 default plate
The day variant is derived as <room>_day_v1.png regardless of the source plate's revision. The default PLATE is crypt_firelit_v2.png (v2), so a day switch to crypt_day_v1.png would load a different art revision than the curated v2 plate. This is gated by an explicit deploy-written sentinel file and an existence check, so it only fires when a v1 day plate is intentionally present — but the suffix is not derived from (or validated against) the resolved PLATE's revision. If an operator ships only _day_v2.png, the day state silently falls through to the night plate. Consider deriving the revision from the source PLATE (e.g. capture the trailing _vN) or documenting the _v1 convention where the sentinel is written.
Category: Unity scene/prefab
Why this matters: A room whose curated plate is v2 but whose only available day art is v2 would get no day plate at all (silent fallback to night), or a v1 day plate of inconsistent quality — both are silent operator surprises rather than errors. Low blast radius since it's additive and default-off.
evaOS review status: completedPR: #1298 - renderer: day-state plate selection (#1291) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1298 Review URL: #1298 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #1298 - renderer: day-state plate selection (#1291)
Head: e69f3d5d5bebdf73020d547089f91a945040c0ed into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
extensions/renderers/unity/scripts/paint_combat_v1.cs |
modified | +19/-0 | Unity/gameplay state | Moderate: validated P2 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 1, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Runtime correctness: 1
- Unity scene/prefab: 2
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - Unity asset/script/project files changed. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1291.
Suggested labels: unity.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
Assets/**- Prioritize scene, prefab, save-state, asset-reference, and gameplay regressions. - Path instructions:
ProjectSettings/**- Treat build, platform, input, graphics, and release behavior changes as high risk. - Label suggestions: unity, gameplay, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
| if(_tod=="day"){ | ||
| var _us=PLATE.IndexOf('_'); | ||
| var _room=_us>0?PLATE.Substring(0,_us):PLATE.Replace(".png",""); | ||
| var _dayName=_room+"_day_v1.png"; |
There was a problem hiding this comment.
P2: Day-plate filename convention is hardcoded and unverified vs the generator/deploy step
The renderer hardcodes the day sibling as <room>_day_v1.png. The room prefix is derived from the resolved PLATE (everything before the first underscore), so the active night default crypt_firelit_v2.png -> crypt -> looks for crypt_day_v1.png. But (a) the generator half (generate_room.py --day) and the deploy/driver that actually writes plates into Assets/painterly/backdrops/ are NOT in this diff and no tracked file enforces the _day_v1.png naming; (b) the _v1 suffix is hardcoded while the night plate is already on _v2, so a v2+ day plate (or any non-_day_v1.png name like tavern_layered_day_v1.png) silently never matches and the feature is an invisible no-op. If the deploy step names day plates differently, day-state selection fails safe to night with zero diagnostic. Either resolve the day plate via the SAME plate map the night plate uses (_location_plates.json, extended with a day variant), or assert/document the exact deploy-time filename contract and emit a one-line sb.AppendLine when sentinel==day but no day sibling is found so a misnamed plate is diagnosable instead of silent.
Category: Unity scene/prefab
Why this matters: A day/night render feature that silently never activates because of an unverified filename coupling is a classic release-regression/proof-gap: it looks wired but produces no visible day evidence, and the absence of evidence is indistinguishable from 'sentinel not set'.
| var _room=_us>0?PLATE.Substring(0,_us):PLATE.Replace(".png",""); | ||
| var _dayName=_room+"_day_v1.png"; | ||
| var _dayPath="Assets/painterly/backdrops/"+_dayName; | ||
| if(System.IO.File.Exists("/home/unity/worldos-unity/"+_dayPath)) PLATE=_dayName; |
There was a problem hiding this comment.
P3: No editor/fixture smoke evidence for day-state render behavior in this diff
This PR adds runtime render behavior (conditional day-plate selection that changes the painted backdrop) but the diff carries no focused Unity editor smoke, fixture, or a captured day-vs-night plate pair proving the sentinel path resolves and renders the day sibling. Repo proof expectations call for editor/play-mode/fixture evidence when runtime behavior changes. No tracked test exercises this branch (qa/* time_of_day references are engine state, not this renderer sentinel). Add at minimum a one-off editor capture log showing sentinel=day + present *_day_v1.png -> PLATE reassigned and rendered, and sentinel absent/night -> unchanged, so the additive/default-off contract is evidenced rather than asserted.
Category: Unity scene/prefab
| var _tod=System.IO.File.ReadAllText(_todF).Trim().ToLowerInvariant(); | ||
| if(_tod=="day"){ | ||
| var _us=PLATE.IndexOf('_'); | ||
| var _room=_us>0?PLATE.Substring(0,_us):PLATE.Replace(".png",""); |
There was a problem hiding this comment.
P3: Room-prefix extraction drops plate version; day lookup is pinned to v1 regardless of night version
_room is PLATE.Substring(0,_us) — for crypt_firelit_v2.png this yields crypt (correct), but the version segment (firelit_v2) is discarded and the day name is rebuilt as crypt_day_v1.png with a hardcoded _v1. As night plates advance to v2/v3, their day counterparts stay pinned to v1 by this code, so a freshly regenerated day plate at a higher version would be ignored. At minimum, if the _day_v1.png convention is intentional, add a comment stating the version is intentionally fixed; otherwise derive the day name from the full non-extension stem or a registry field.
Category: Runtime correctness
1 participant
Summary
paint_combat_v1.cs's existing plate-resolution block (right after the_location_plates.json/_active_combat.txtprecedence chain, ~line 27)._time_of_day.txtsentinel on the box (mirrors the existing_active_combat.txtconfig-file pattern) saysday/night. When it saysdayand a naming-convention sibling<room>_day_v1.pngexists (room = the resolved plate's prefix up to the first underscore, e.g.tavern_layered_v1.png->tavern), the renderer prefers the day plate.day, or a missing day plate all fall through unchanged to today's night/current behavior — default is unchanged.campaign.time_of_day(already tracked,servers/engine/travel.pyPHASES) is not yet surfaced on/combat-surface(build_combat_surfaceinviewer/server.pyhas notime_of_dayfield), so this wires via the box-side sentinel rather than adding a new engine-surfaced field — kept as the minimal honest wiring for [graphics] Day/night live integration — deploy --lighting day variants + time-of-day plate swap on the box #1291's generation-done/wiring-open half. A future pass could write the sentinel automatically from the engine'stime_of_dayinstead of a manual/driver-set file.Test plan
git apply/compile validated via box-sideexecute_code(roslyn) hand-merge in the box editor for live validation — see [graphics] Day/night live integration — deploy --lighting day variants + time-of-day plate swap on the box #1291 comment for frame evidence.