-
Notifications
You must be signed in to change notification settings - Fork 131
remove not needed renderer #460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
WalkthroughLarge-scale monorepo refactoring: removes the entire Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 29
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| "start": "run-p dev-proxy dev-rsbuild watch-mesher", | ||
| "start2": "run-p dev-rsbuild watch-mesher", | ||
| "start": "run-p dev-proxy dev-rsbuild", | ||
| "start2": "run-p dev-rsbuild", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Build script references removed build-other-workers script
The build and build-analyze scripts reference pnpm build-other-workers, but this PR removes the build-other-workers script definition (which was "echo NOT IMPLEMENTED"). Running pnpm build will now fail because the build-other-workers script no longer exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CONTRIBUTING.md (1)
46-54: Align unit-test docs with renderer removalThe heading rename on Line 46 looks good, but the unit tests section still says:
Location of unit tests:
**/*.test.tsfiles insrcfolder andrendererfolder.Given the
rendererpackage/folder is being removed in this PR, this sentence is now stale and may confuse contributors. Consider updating it to only mention the actual test locations.package.json (1)
8-9: Scripts and renderer dependency wiring look coherent; consider small cleanup
- The updated
start/start2scripts and the addition of theminecraft-rendererdependency (withthree/@types/threepinned via devDeps + overrides) are consistent with the new imports inexperiments/*. No issues from a manifest perspective.- Minor nit: the eslint script on Line 23 still globs
renderereven though that package/folder is removed in this PR. It’s harmless but a bit misleading; you might want to droprendererfrom the glob in a follow-up for clarity.- "lint": "eslint \"{src,cypress,renderer}/**/*.{ts,js,jsx,tsx}\"", + "lint": "eslint \"{src,cypress}/**/*.{ts,js,jsx,tsx}\"",Please re-run your usual
pnpm lint/ dev build to confirm the newminecraft-rendererdependency and its/src/...subpath imports resolve as expected in your environment.Also applies to: 23-23, 84-85, 132-132, 197-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (39)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrenderer/viewer/three/entity/models/allay.objis excluded by!**/*.objrenderer/viewer/three/entity/models/arrow.objis excluded by!**/*.objrenderer/viewer/three/entity/models/axolotl.objis excluded by!**/*.objrenderer/viewer/three/entity/models/blaze.objis excluded by!**/*.objrenderer/viewer/three/entity/models/boat.objis excluded by!**/*.objrenderer/viewer/three/entity/models/camel.objis excluded by!**/*.objrenderer/viewer/three/entity/models/cat.objis excluded by!**/*.objrenderer/viewer/three/entity/models/chicken.objis excluded by!**/*.objrenderer/viewer/three/entity/models/cod.objis excluded by!**/*.objrenderer/viewer/three/entity/models/creeper.objis excluded by!**/*.objrenderer/viewer/three/entity/models/dolphin.objis excluded by!**/*.objrenderer/viewer/three/entity/models/ender_dragon.objis excluded by!**/*.objrenderer/viewer/three/entity/models/enderman.objis excluded by!**/*.objrenderer/viewer/three/entity/models/endermite.objis excluded by!**/*.objrenderer/viewer/three/entity/models/fox.objis excluded by!**/*.objrenderer/viewer/three/entity/models/frog.objis excluded by!**/*.objrenderer/viewer/three/entity/models/ghast.objis excluded by!**/*.objrenderer/viewer/three/entity/models/goat.objis excluded by!**/*.objrenderer/viewer/three/entity/models/guardian.objis excluded by!**/*.objrenderer/viewer/three/entity/models/horse.objis excluded by!**/*.objrenderer/viewer/three/entity/models/llama.objis excluded by!**/*.objrenderer/viewer/three/entity/models/minecart.objis excluded by!**/*.objrenderer/viewer/three/entity/models/parrot.objis excluded by!**/*.objrenderer/viewer/three/entity/models/piglin.objis excluded by!**/*.objrenderer/viewer/three/entity/models/pillager.objis excluded by!**/*.objrenderer/viewer/three/entity/models/rabbit.objis excluded by!**/*.objrenderer/viewer/three/entity/models/sheep.objis excluded by!**/*.objrenderer/viewer/three/entity/models/shulker.objis excluded by!**/*.objrenderer/viewer/three/entity/models/sniffer.objis excluded by!**/*.objrenderer/viewer/three/entity/models/spider.objis excluded by!**/*.objrenderer/viewer/three/entity/models/tadpole.objis excluded by!**/*.objrenderer/viewer/three/entity/models/turtle.objis excluded by!**/*.objrenderer/viewer/three/entity/models/vex.objis excluded by!**/*.objrenderer/viewer/three/entity/models/villager.objis excluded by!**/*.objrenderer/viewer/three/entity/models/warden.objis excluded by!**/*.objrenderer/viewer/three/entity/models/witch.objis excluded by!**/*.objrenderer/viewer/three/entity/models/wolf.objis excluded by!**/*.objrenderer/viewer/three/entity/models/zombie_villager.objis excluded by!**/*.obj
📒 Files selected for processing (81)
CONTRIBUTING.md(1 hunks)config.json(1 hunks)experiments/state.ts(1 hunks)experiments/three-fireworks.ts(1 hunks)experiments/three-item.ts(1 hunks)experiments/three-labels.ts(1 hunks)package.json(3 hunks)pnpm-workspace.yaml(0 hunks)renderer/.npmrc(0 hunks)renderer/buildMesherConfig.mjs(0 hunks)renderer/buildMesherWorker.mjs(0 hunks)renderer/package.json(0 hunks)renderer/playground.html(0 hunks)renderer/playground/allEntitiesDebug.ts(0 hunks)renderer/playground/baseScene.ts(0 hunks)renderer/playground/playground.ts(0 hunks)renderer/playground/playgroundUi.tsx(0 hunks)renderer/playground/scenes/allEntities.ts(0 hunks)renderer/playground/scenes/entities.ts(0 hunks)renderer/playground/scenes/floorRandom.ts(0 hunks)renderer/playground/scenes/frequentUpdates.ts(0 hunks)renderer/playground/scenes/geometryExport.ts(0 hunks)renderer/playground/scenes/index.ts(0 hunks)renderer/playground/scenes/lightingStarfield.ts(0 hunks)renderer/playground/scenes/main.ts(0 hunks)renderer/playground/scenes/railsCobweb.ts(0 hunks)renderer/playground/scenes/rotationIssue.ts(0 hunks)renderer/playground/scenes/slabsOptimization.ts(0 hunks)renderer/playground/scenes/transparencyIssue.ts(0 hunks)renderer/playground/shared.ts(0 hunks)renderer/rsbuild.config.ts(0 hunks)renderer/viewer/.gitignore(0 hunks)renderer/viewer/baseGraphicsBackend.ts(0 hunks)renderer/viewer/common/utils.ts(0 hunks)renderer/viewer/lib/DebugGui.ts(0 hunks)renderer/viewer/lib/animationController.ts(0 hunks)renderer/viewer/lib/basePlayerState.ts(0 hunks)renderer/viewer/lib/cameraBobbing.ts(0 hunks)renderer/viewer/lib/cleanupDecorator.ts(0 hunks)renderer/viewer/lib/createPlayerObject.ts(0 hunks)renderer/viewer/lib/guiRenderer.ts(0 hunks)renderer/viewer/lib/mesher/mesher.ts(0 hunks)renderer/viewer/lib/mesher/models.ts(0 hunks)renderer/viewer/lib/mesher/modelsGeometryCommon.ts(0 hunks)renderer/viewer/lib/mesher/shared.ts(0 hunks)renderer/viewer/lib/mesher/standaloneRenderer.ts(0 hunks)renderer/viewer/lib/mesher/test/mesherTester.ts(0 hunks)renderer/viewer/lib/mesher/test/playground.ts(0 hunks)renderer/viewer/lib/mesher/test/tests.test.ts(0 hunks)renderer/viewer/lib/mesher/world.ts(0 hunks)renderer/viewer/lib/mesher/worldConstants.ts(0 hunks)renderer/viewer/lib/mesherlogReader.ts(0 hunks)renderer/viewer/lib/moreBlockDataGenerated.json(0 hunks)renderer/viewer/lib/simpleUtils.ts(0 hunks)renderer/viewer/lib/skyLight.ts(0 hunks)renderer/viewer/lib/smoothSwitcher.ts(0 hunks)renderer/viewer/lib/ui/newStats.ts(0 hunks)renderer/viewer/lib/utils.ts(0 hunks)renderer/viewer/lib/utils/proxy.ts(0 hunks)renderer/viewer/lib/utils/skins.ts(0 hunks)renderer/viewer/lib/workerProxy.ts(0 hunks)renderer/viewer/lib/worldDataEmitter.ts(0 hunks)renderer/viewer/lib/worldrendererCommon.ts(0 hunks)renderer/viewer/sign-renderer/index.html(0 hunks)renderer/viewer/sign-renderer/index.ts(0 hunks)renderer/viewer/sign-renderer/noop.js(0 hunks)renderer/viewer/sign-renderer/package.json(0 hunks)renderer/viewer/sign-renderer/playground.ts(0 hunks)renderer/viewer/sign-renderer/tests.test.ts(0 hunks)renderer/viewer/sign-renderer/vite.config.ts(0 hunks)renderer/viewer/three/appShared.ts(0 hunks)renderer/viewer/three/bannerRenderer.ts(0 hunks)renderer/viewer/three/cameraShake.ts(0 hunks)renderer/viewer/three/documentRenderer.ts(0 hunks)renderer/viewer/three/entities.ts(0 hunks)renderer/viewer/three/entity/EntityMesh.ts(0 hunks)renderer/viewer/three/entity/animations.js(0 hunks)renderer/viewer/three/entity/armorModels.json(0 hunks)renderer/viewer/three/entity/armorModels.ts(0 hunks)renderer/viewer/three/entity/exportedModels.js(0 hunks)renderer/viewer/three/entity/externalTextures.json(0 hunks)
💤 Files with no reviewable changes (74)
- renderer/package.json
- renderer/viewer/lib/cleanupDecorator.ts
- renderer/buildMesherConfig.mjs
- renderer/viewer/baseGraphicsBackend.ts
- renderer/playground/allEntitiesDebug.ts
- renderer/viewer/lib/mesher/worldConstants.ts
- renderer/viewer/sign-renderer/noop.js
- renderer/viewer/lib/mesher/test/playground.ts
- renderer/viewer/lib/mesher/test/mesherTester.ts
- renderer/playground/scenes/allEntities.ts
- renderer/viewer/lib/animationController.ts
- renderer/viewer/common/utils.ts
- renderer/viewer/three/documentRenderer.ts
- renderer/viewer/lib/smoothSwitcher.ts
- renderer/playground/scenes/geometryExport.ts
- renderer/viewer/three/entity/armorModels.json
- renderer/playground/scenes/index.ts
- renderer/viewer/lib/simpleUtils.ts
- renderer/viewer/lib/workerProxy.ts
- renderer/playground/scenes/railsCobweb.ts
- renderer/playground/playground.ts
- renderer/.npmrc
- renderer/playground/scenes/rotationIssue.ts
- renderer/viewer/sign-renderer/vite.config.ts
- renderer/viewer/lib/mesher/mesher.ts
- renderer/viewer/lib/moreBlockDataGenerated.json
- renderer/viewer/three/entity/EntityMesh.ts
- renderer/playground.html
- renderer/buildMesherWorker.mjs
- renderer/playground/scenes/main.ts
- renderer/viewer/lib/DebugGui.ts
- renderer/viewer/three/appShared.ts
- renderer/playground/scenes/entities.ts
- renderer/viewer/lib/mesher/models.ts
- renderer/playground/scenes/frequentUpdates.ts
- pnpm-workspace.yaml
- renderer/viewer/three/bannerRenderer.ts
- renderer/viewer/lib/mesher/modelsGeometryCommon.ts
- renderer/viewer/lib/mesherlogReader.ts
- renderer/viewer/lib/basePlayerState.ts
- renderer/viewer/lib/mesher/shared.ts
- renderer/viewer/lib/guiRenderer.ts
- renderer/viewer/three/entity/externalTextures.json
- renderer/viewer/three/entity/armorModels.ts
- renderer/viewer/lib/createPlayerObject.ts
- renderer/viewer/sign-renderer/package.json
- renderer/viewer/three/entity/animations.js
- renderer/viewer/lib/utils.ts
- renderer/viewer/sign-renderer/index.html
- renderer/rsbuild.config.ts
- renderer/viewer/lib/mesher/world.ts
- renderer/viewer/three/cameraShake.ts
- renderer/viewer/sign-renderer/tests.test.ts
- renderer/playground/playgroundUi.tsx
- renderer/viewer/lib/mesher/standaloneRenderer.ts
- renderer/playground/scenes/transparencyIssue.ts
- renderer/viewer/lib/mesher/test/tests.test.ts
- renderer/viewer/three/entities.ts
- renderer/viewer/sign-renderer/playground.ts
- renderer/viewer/lib/worldrendererCommon.ts
- renderer/viewer/lib/skyLight.ts
- renderer/playground/scenes/slabsOptimization.ts
- renderer/viewer/lib/utils/skins.ts
- renderer/viewer/lib/utils/proxy.ts
- renderer/viewer/.gitignore
- renderer/viewer/lib/worldDataEmitter.ts
- renderer/viewer/lib/ui/newStats.ts
- renderer/playground/scenes/lightingStarfield.ts
- renderer/playground/shared.ts
- renderer/viewer/lib/cameraBobbing.ts
- renderer/playground/scenes/floorRandom.ts
- renderer/viewer/three/entity/exportedModels.js
- renderer/playground/baseScene.ts
- renderer/viewer/sign-renderer/index.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: zardoy
Repo: zardoy/minecraft-web-client PR: 373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, team information for entities should be inlined into entity update events from the world data emitter rather than accessing the global bot object from renderer code. This maintains better separation of concerns between the data layer and renderer.
Learnt from: zardoy
Repo: zardoy/minecraft-web-client PR: 373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, files under renderer/ directory must not access the global `bot` variable directly according to .cursor/rules/vars-usage.mdc. The updateNameTagVisibility method in renderer/viewer/three/entities.ts currently violates this rule by accessing bot.teamMap and bot.username. Team information should be passed through entity update events from the world data emitter instead.
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to renderer/**/*.ts : Do not use the global variable `bot` directly in any file under the `renderer/` directory or its subfolders (e.g., `renderer/viewer/three/worldrendererThree.ts`).
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to renderer/**/*.ts : In renderer code (such as in `WorldRendererThree`), use the `playerState` property (e.g., `worldRenderer.playerState.gameMode`) to access player state. The implementation for `playerState` lives in `src/mineflayer/playerState.ts`.
Applied to files:
experiments/state.tsexperiments/three-labels.tsCONTRIBUTING.md
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to renderer/**/*.ts : In renderer code, all bot/player state and events must be accessed via explicit interfaces, state managers, or passed-in objects, never by referencing `bot` directly.
Applied to files:
experiments/state.ts
📚 Learning: 2025-06-23T13:33:14.776Z
Learnt from: zardoy
Repo: zardoy/minecraft-web-client PR: 373
File: renderer/viewer/three/entities.ts:1120-1120
Timestamp: 2025-06-23T13:33:14.776Z
Learning: In the minecraft-web-client project, files under renderer/ directory must not access the global `bot` variable directly according to .cursor/rules/vars-usage.mdc. The updateNameTagVisibility method in renderer/viewer/three/entities.ts currently violates this rule by accessing bot.teamMap and bot.username. Team information should be passed through entity update events from the world data emitter instead.
Applied to files:
experiments/state.tsexperiments/three-fireworks.tsexperiments/three-item.ts
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to renderer/**/*.ts : Do not use the global variable `bot` directly in any file under the `renderer/` directory or its subfolders (e.g., `renderer/viewer/three/worldrendererThree.ts`).
Applied to files:
experiments/state.tsexperiments/three-fireworks.tsexperiments/three-item.tsCONTRIBUTING.md
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to src/**/*.ts : In `src/` code, you may use the global variable `appViewer` from `src/appViewer.ts` directly. Do not import `appViewer` or use `window.appViewer`; use the global `appViewer` variable as-is.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
Repo: zardoy/minecraft-web-client PR: 0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-27T13:24:58.186Z
Learning: Applies to src/**/*.ts : You may use the global variable `bot` directly in any file under the `src/` directory (e.g., `src/mineflayer/playerState.ts`).
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-07-10T16:03:10.838Z
Learnt from: zardoy
Repo: zardoy/minecraft-web-client PR: 391
File: renderer/viewer/three/threeJsUtils.ts:56-56
Timestamp: 2025-07-10T16:03:10.838Z
Learning: The minecraft-web-client project uses core-js auto polyfill injection, which means newer JavaScript features like Promise.withResolvers() are automatically polyfilled for compatibility across different environments.
Applied to files:
package.json
⏰ 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: Cursor Bugbot
🔇 Additional comments (5)
config.json (1)
34-35: Splash text copy change is finePure text update on Line 34, JSON structure unchanged and consistent with existing keys.
experiments/three-labels.ts (1)
3-3: Import rewire to shared renderer utility looks goodThe import on Line 3 now points at the shared
waypointSpriteutil from the external renderer package; no other behavior change in this file. As long as that subpath is exported by the package, this aligns with the rest of the PR.Please run the experiments build (Vite config) to confirm the bundler resolves
minecraft-renderer/src/three/waypointSpritecorrectly.experiments/three-item.ts (1)
4-4: ExternalizingitemMeshimport is consistent with the new renderer packageThe switch to importing
createItemMeshFromCanvas/createItemMeshfrom the external renderer module keeps the existing call sites intact, so runtime behavior should remain the same if the API surface matches.Double-check that the
minecraft-rendererversion you pinned actually exportssrc/three/itemMeshwith the same function signatures used here.experiments/three-fireworks.ts (1)
3-3: Fireworks manager import migration matches renderer extractionRepointing
FireworksManagerto the external renderer package on Line 3 keeps the surrounding usage identical and aligns with the rest of the refactor.Please confirm that
minecraft-rendererexposessrc/three/fireworksand that the experiments build runs without module resolution errors.experiments/state.ts (1)
1-1: SmoothSwitcher import source change is straightforwardImporting
SmoothSwitcherfrom the shared renderer package on Line 1 is consistent with the rest of the PR; the usage pattern in this experiment remains unchanged.After bumping the import, ensure this experiment still runs and that
minecraft-renderer/src/lib/smoothSwitcheris present in the installed package.
Note
Replace internal three.js renderer with external
minecraft-renderer, refactor imports and app/bootstrap, and update build/test configs while removing old viewer code.renderer/viewer/three/**, panorama, item mesh, skybox, media, particles, waypoints, etc.).minecraft-rendererpackage; refactor imports throughoutsrc/**to use it (viewer, utils, mesher, three methods, player state).rsbuildSharedConfig.tsand update paths; copy mesher worker fromnode_modules.rsbuild.config.ts, vitest root, and docker prepare script; remove renderer-specific adjustments.AppViewerwith one fromminecraft-renderer; addonAppViewerConfigUpdateand adjust initialization/loader logic.Written by Cursor Bugbot for commit 52b99ca. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.