Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
/update |
WalkthroughThe changes add support for a new "sheep" entity in the Changes
Sequence Diagram(s)sequenceDiagram
participant EM as EntityMesh
participant Loader as OBJLoader
participant MeshProc as Mesh Processing
Note over EM: Sheep entity workflow
EM ->> EM: Check entity type ("sheep")
EM ->> EM: Call handleSheep(objLoader, "sheep", overrides)
EM ->> MeshProc: applyMaterialToMesh(mesh, material)
EM ->> MeshProc: applyEntityScaleAndPosition(mesh1, mesh2, "sheep")
EM ->> MeshProc: applyHeadRotation(mesh, rotation)
MeshProc -->> EM: Configured sheep mesh
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
renderer/viewer/lib/entity/EntityMesh.ts (2)
439-507: Extract common rotation logic and constants.Consider the following improvements:
- There's code duplication in head rotation logic between
applyHeadRotationandapplyMaterialAndTransform.- The magic number
180in rotation calculations should be extracted as a constant.+const DEGREES_TO_RADIANS = Math.PI / 180; + +private applyHeadRotationToMesh(mesh: THREE.Object3D, rotation: { x?: number; y?: number; z?: number }): void { + mesh.rotation.x -= (rotation.x ?? 0) * DEGREES_TO_RADIANS; + mesh.rotation.y -= (rotation.y ?? 0) * DEGREES_TO_RADIANS; + mesh.rotation.z -= (rotation.z ?? 0) * DEGREES_TO_RADIANS; +} + private applyHeadRotation(mesh: THREE.Object3D, rotation: { x?: number; y?: number; z?: number }): void { - mesh.rotation.x -= (rotation.x ?? 0) * Math.PI / 180; - mesh.rotation.y -= (rotation.y ?? 0) * Math.PI / 180; - mesh.rotation.z -= (rotation.z ?? 0) * Math.PI / 180; + this.applyHeadRotationToMesh(mesh, rotation); }Update
applyMaterialAndTransformto use the common method:if (child.name === 'Head' && overrides.rotation?.head) { - child.rotation.x -= (overrides.rotation.head.x ?? 0) * Math.PI / 180; - child.rotation.y -= (overrides.rotation.head.y ?? 0) * Math.PI / 180; - child.rotation.z -= (overrides.rotation.head.z ?? 0) * Math.PI / 180; + this.applyHeadRotationToMesh(child, overrides.rotation.head); }
467-487: Make sheep colors configurable through overrides.The colors for sheep and coat materials are hardcoded. Consider making them configurable through the overrides parameter to support different sheep colors.
interface EntityOverrides { textures?: Record<string, string> rotation?: Record<string, { x?: number; y?: number; z?: number }> + colors?: { + sheep?: number + sheepCoat?: number + } } private handleSheep(objLoader: OBJLoader, originalType: string, overrides: EntityOverrides): THREE.Object3D { const sheepObj = objLoader.parse(sheep) - const sheepMaterial = new THREE.MeshLambertMaterial({ color: 0xff_ff_ff }) + const sheepMaterial = new THREE.MeshLambertMaterial({ color: overrides.colors?.sheep ?? 0xff_ff_ff }) this.applyMaterialToMesh(sheepObj, sheepMaterial) const sheepCoatObj = objLoader.parse(sheepCoat) - const sheepCoatMaterial = new THREE.MeshLambertMaterial({ color: 0xff_d7_00 }) + const sheepCoatMaterial = new THREE.MeshLambertMaterial({ color: overrides.colors?.sheepCoat ?? 0xff_d7_00 }) this.applyMaterialToMesh(sheepCoatObj, sheepCoatMaterial)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
renderer/viewer/lib/entity/models/sheep.objis excluded by!**/*.objrenderer/viewer/lib/entity/models/sheepCoat.objis excluded by!**/*.obj
📒 Files selected for processing (2)
renderer/viewer/lib/entity/EntityMesh.ts(5 hunks)renderer/viewer/lib/entity/exportedModels.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (4)
renderer/viewer/lib/entity/exportedModels.js (1)
26-26: LGTM!The new
sheepCoatexport follows the established pattern and maintains alphabetical order.renderer/viewer/lib/entity/EntityMesh.ts (3)
13-13: LGTM!The new imports for sheep models are correctly added.
410-412: LGTM!The scale values for arrow and sheep entities are appropriately set.
526-530: LGTM!The sheep-specific handling in the constructor follows the existing pattern and appropriately returns early after handling.
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-em82ljayk-zaro.vercel.app |
2 participants
Summary by CodeRabbit