a little less fog #426
a little less fog #426
Conversation
WalkthroughSkyboxRenderer.update gains an optional partialTicks parameter and stores it internally for interpolation. Time-dependent helpers now consume internal state (no-arg getFogColor/getSkyColor/getCelestialAngle). The fog distance factor increases to 2.5× view distance. Immediate sky color updates on viewDistance change are removed; color updates occur in the normal update/render flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant SkyboxRenderer as SkyboxRenderer
participant Scene as Three.js Scene
App->>SkyboxRenderer: update(cameraPos, viewDistance, partialTicks?)
activate SkyboxRenderer
SkyboxRenderer->>SkyboxRenderer: this.partialTicks = partialTicks || 0
SkyboxRenderer->>SkyboxRenderer: store cameraPos / viewDistance
note right of SkyboxRenderer: No immediate updateSkyColors on distance change
App->>SkyboxRenderer: render()
SkyboxRenderer->>SkyboxRenderer: angle = getCelestialAngle()
SkyboxRenderer->>SkyboxRenderer: skyColor = getSkyColor()
SkyboxRenderer->>SkyboxRenderer: fogColor = getFogColor()
SkyboxRenderer->>SkyboxRenderer: compute fog distances (2.5× viewDistance)
SkyboxRenderer->>Scene: apply sky, fog, underwater paths
deactivate SkyboxRenderer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
dont review |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-ld46awkmx-zaro.vercel.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
renderer/viewer/three/skyboxRenderer.ts (2)
281-283: Fog stops updating with textured skybox (early return blocks it).
updateSkyColors()bails out whenskyMesh/voidMeshare absent, so in textured skybox mode (where onlythis.meshexists) fog/background never update and partialTicks interpolation is ineffective.Apply this diff to decouple fog/background updates from gradient-mesh presence and guard mesh-only color updates:
private updateSkyColors () { - if (!this.skyMesh || !this.voidMesh) return + // Allow fog/background updates even when gradient meshes are not present. @@ if (this.inWater) { const waterViewDistance = this.waterBreathing ? 100 : 5 this.scene.fog = new THREE.Fog(new THREE.Color(0, 0, 1), 0.0025, waterViewDistance) this.scene.background = new THREE.Color(0, 0, 1) // Update sky and void colors for underwater effect - ;(this.skyMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(0, 0, 1)) - ;(this.voidMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(0, 0, 0.6)) + if (this.skyMesh) (this.skyMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(0, 0, 1)) + if (this.voidMesh) (this.voidMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(0, 0, 0.6)) return } @@ - ;(this.skyMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(skyColor.x, skyColor.y, skyColor.z)) - ;(this.voidMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color( - skyColor.x * 0.2 + 0.04, - skyColor.y * 0.2 + 0.04, - skyColor.z * 0.6 + 0.1 - )) + if (this.skyMesh) (this.skyMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color(skyColor.x, skyColor.y, skyColor.z)) + if (this.voidMesh) (this.voidMesh.material as THREE.MeshBasicMaterial).color.set(new THREE.Color( + skyColor.x * 0.2 + 0.04, + skyColor.y * 0.2 + 0.04, + skyColor.z * 0.6 + 0.1 + ))Also applies to: 291-301, 324-330
198-213: Celestial angle smoothing is a no-op.
angle += (angle - angle) / 3always adds 0. Use the smoothed value in the delta.- angle = 1 - ((Math.cos(angle * Math.PI) + 1) / 2) - angle += (angle - angle) / 3 + const smooth = 1 - ((Math.cos(angle * Math.PI) + 1) / 2) + angle += (smooth - angle) / 3
🧹 Nitpick comments (3)
renderer/viewer/three/skyboxRenderer.ts (3)
161-176: Avoid per-frame allocations for fog color.
new THREE.Vector3each call allocates. Consider caching vectors on the instance and mutating them.
177-196: Use Math.PI consistently; default params make (0,0) redundant.
- Replace literal
3.141_593withMath.PI.- Since
x/zdefault to 0, callers can omit them.- let brightness = Math.cos(angle * 3.141_593 * 2) * 2 + 0.5 + let brightness = Math.cos(angle * Math.PI * 2) * 2 + 0.5
307-310: Tiny cleanup: drop explicit zeros.
getSkyColor()defaultsx/zto 0; call without args.- const skyColor = this.getSkyColor(0, 0) + const skyColor = this.getSkyColor()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
renderer/viewer/three/skyboxRenderer.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
renderer/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/vars-usage.mdc)
renderer/**/*.ts: Do not use the global variablebotdirectly in any file under therenderer/directory or its subfolders (e.g.,renderer/viewer/three/worldrendererThree.ts).
In renderer code, all bot/player state and events must be accessed via explicit interfaces, state managers, or passed-in objects, never by referencingbotdirectly.
In renderer code (such as inWorldRendererThree), use theplayerStateproperty (e.g.,worldRenderer.playerState.gameMode) to access player state. The implementation forplayerStatelives insrc/mineflayer/playerState.ts.
Files:
renderer/viewer/three/skyboxRenderer.ts
⏰ 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: build-and-deploy
🔇 Additional comments (3)
renderer/viewer/three/skyboxRenderer.ts (3)
79-86: Confirm partialTicks source of truth.Both
update()andupdateTime()assignthis.partialTicks. Ensure call order is stable per frame to avoid jitter. Consider updating partialTicks only in one place (preferablyupdate()).
215-217: Nice simplification.Centralizing partialTicks consumption via
getCelestialAngle()improves call sites and reduces parameter threading.
322-322: Verify fog distance tweak (×2.5) against view distance units.If
viewDistanceis in chunks (×16 later), confirm visual parity across render distances and no popping. Consider near value scaling with distance as well.
| if (this.mesh) { | ||
| // Update skybox position | ||
| this.mesh.position.copy(cameraPosition) | ||
| } else if (this.skyMesh) { | ||
| // Update gradient sky position | ||
| this.skyMesh.position.copy(cameraPosition) | ||
| this.voidMesh?.position.copy(cameraPosition) | ||
| this.updateSkyColors() // Update colors based on time of day | ||
| this.updateSkyColors() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update fog/colors every frame to leverage partialTicks.
Right now updateSkyColors() is only called on the gradient path. Call it unconditionally each frame so fog interpolation works with both gradient and textured skyboxes.
} else if (this.skyMesh) {
// Update gradient sky position
this.skyMesh.position.copy(cameraPosition)
this.voidMesh?.position.copy(cameraPosition)
- this.updateSkyColors()
}
+ this.updateSkyColors()📝 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.
| if (this.mesh) { | |
| // Update skybox position | |
| this.mesh.position.copy(cameraPosition) | |
| } else if (this.skyMesh) { | |
| // Update gradient sky position | |
| this.skyMesh.position.copy(cameraPosition) | |
| this.voidMesh?.position.copy(cameraPosition) | |
| this.updateSkyColors() // Update colors based on time of day | |
| this.updateSkyColors() | |
| } | |
| } | |
| if (this.mesh) { | |
| // Update skybox position | |
| this.mesh.position.copy(cameraPosition) | |
| } else if (this.skyMesh) { | |
| // Update gradient sky position | |
| this.skyMesh.position.copy(cameraPosition) | |
| this.voidMesh?.position.copy(cameraPosition) | |
| } | |
| this.updateSkyColors() | |
| } |
🤖 Prompt for AI Agents
In renderer/viewer/three/skyboxRenderer.ts around lines 87 to 96,
updateSkyColors() is only called for the gradient path so fog/color
interpolation using partialTicks is not applied for textured skyboxes; move or
call updateSkyColors() unconditionally each frame (i.e., call it after the
mesh/skyMesh position updates or ensure both branches invoke it) so fog and
color interpolation run every frame for both gradient and textured skyboxes.
1 participant
Summary by CodeRabbit