feat: Refactor mouse controls, fixing all false entity/item interaction issues #286
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces a new dependency, transitions mouse interaction logic from legacy world interactions to a dedicated mouse plugin, and updates various event handlers to utilize the new functionality. Modifications include the removal of obsolete constants and functions, refinement of type declarations, addition of null checks, and support for a new configuration option for precise mouse input. A new file implements a display manager and event listeners for enhanced mouse interactions, while the outdated world interactions file has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CI as Controller (Camera/GameOverlay/Touch)
participant B as Bot
participant MP as Mouse Plugin
participant DM as Display Manager
U->>CI: Initiates mouse/touch event
CI->>B: Calls bot.mouse.update()
B->>MP: Forwards update event
MP->>DM: Updates cursor state and animations
DM-->>U: Renders visual feedback
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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 (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/makeOptimizedMcData.mjs(1 hunks)src/sounds/botSoundSystem.ts(2 hunks)src/sounds/soundsMap.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- scripts/makeOptimizedMcData.mjs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (4)
src/sounds/botSoundSystem.ts (1)
24-24:❓ Verification inconclusive
Verify the safety of removing the sounds data check.
The condition has been simplified to only check
miscUiState.gameLoaded. Please verify that removing theloadedData.soundscheck won't cause issues if sound data isn't loaded when this executes.
🏁 Script executed:
#!/bin/bash # Description: Check if loadedData.sounds is used elsewhere in the codebase # to understand potential dependencies rg "loadedData\.sounds" -A 5Length of output: 30
Ensure sound data is safely decoupled from the game loading check.
The updated condition now only verifies whether
miscUiState.gameLoadedis true. An initial search for references toloadedData.soundsreturned no results, which suggests that there aren’t any explicit dependencies on that check elsewhere in the codebase. However, since the automated search output was inconclusive, please perform a manual verification to confirm that this change won’t lead to scenarios where sound data is needed but remains uninitialized.src/sounds/soundsMap.ts (3)
51-51: Well-structured sound mapping implementation!The new implementation provides better organization and explicit error handling:
- Separates ID-to-name and name-to-sounds mappings
- Adds proper type checking for volume and weight
- Uses clear variable names for ID parts
Also applies to: 64-83
264-284: Improved block categorization!The changes enhance the organization and accuracy of block sound mappings:
- Added logical grouping for dirt and ground blocks
- Added proper categorization for crop blocks
- Fixed
nether_wartcategorization by moving it to crop blocks
1-426:❓ Verification inconclusive
Verify PR title accuracy.
The PR title mentions "Refactor mouse controls, fixing all false entity interaction issues" but the changes appear to focus on sound handling improvements. Please verify if this PR description accurately reflects the changes or if there are missing files related to mouse controls.
🏁 Script executed:
#!/bin/bash # Description: Look for related mouse control changes # that might be missing from this PR rg -g '!*.md' -i "mouse.*control" -A 5Length of output: 40
PR Title Mismatch: Mouse Control Changes Not Evident
The changes in
src/sounds/soundsMap.tsare focused solely on sound handling improvements, yet the PR title references "Refactor mouse controls, fixing all false entity interaction issues." Our initial search did not reveal any modifications related to mouse control functionality. Please verify whether (a) the PR title accurately reflects the changes in this commit or (b) additional files related to mouse controls should be included.
src/sounds/botSoundSystem.ts
Outdated
| @@ -108,8 +108,7 @@ subscribeKey(miscUiState, 'gameLoaded', async () => { | |||
| }) | |||
|
|
|||
| bot.on('hardcodedSoundEffectHeard', async (soundIdNum, soundCategory, position, volume, pitch) => { | |||
| const fixOffset = versionToNumber('1.20.4') === versionToNumber(bot.version) ? -1 : 0 | |||
| const soundKey = loadedData.sounds[soundIdNum + fixOffset]?.name | |||
| const soundKey = soundMap!.soundsIdToName[soundIdNum] | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace non-null assertion with proper null check.
Using the non-null assertion operator (!) is unsafe as it assumes soundMap will always be defined. Consider adding a proper null check.
- const soundKey = soundMap!.soundsIdToName[soundIdNum]
+ if (!soundMap) return
+ const soundKey = soundMap.soundsIdToName[soundIdNum]📝 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.
| const soundKey = soundMap!.soundsIdToName[soundIdNum] | |
| if (!soundMap) return | |
| const soundKey = soundMap.soundsIdToName[soundIdNum] |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/mineflayer/plugins/mouse.ts (3)
10-20: Consider consolidating block destruction textures into a single atlas.
Loading each stage texture individually may impact performance. If possible, creating an atlas could reduce texture load requests and streamline rendering.
117-120: Avoid using @ts-expect-error unless strictly necessary.
Relying on@ts-expect-errorsuppresses type safety, making it harder to catch subtle bugs. Consider casting or refining the underlying types formaterial.mapandmaterial.needsUpdate.
140-147: Implement highlight logic for null cursor data.
Currently, ifdatais not provided, there's no fallback or clearing logic for cursor highlights. Implement appropriate handling to ensure the UI state remains consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.json(1 hunks)src/cameraRotationControls.ts(1 hunks)src/devtools.ts(1 hunks)src/index.ts(2 hunks)src/mineflayer/plugins/mouse.ts(1 hunks)src/optionsStorage.ts(1 hunks)src/react/GameInteractionOverlay.tsx(1 hunks)src/react/TouchAreasControls.tsx(3 hunks)src/worldInteractions.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/worldInteractions.ts
🔇 Additional comments (10)
src/mineflayer/plugins/mouse.ts (2)
28-34: Verify external declarations.
beforeRenderFrameandviewerreferences are used but not declared or imported in this file. Ensure they are globally available or imported to avoid runtime reference errors.
178-192: Recheck mouse button usage.
Standard HTML button codes are0for left click,1for middle click, and2for right click. Here,button === 0triggersrightClickStart(), which seems reversed. Confirm the intended mouse handling, as this may confuse users.src/cameraRotationControls.ts (1)
23-23: Potential unintended camera update skip.
if (now - lastMouseMove < 4 && !options.preciseMouseInput) returnmay skip CPU frames when the user moves the mouse rapidly. Confirm that skipping these frames won't introduce jitter or sluggish controls.src/optionsStorage.ts (1)
61-61:❓ Verification inconclusive
Verify the integration of the new mouse input option.
The new
preciseMouseInputoption is well-placed among other mouse-related settings. Please ensure that:
- The option is properly handled in the mouse control logic.
- The UI includes a toggle for this setting.
- Documentation is updated to explain this feature.
🏁 Script executed:
#!/bin/bash # Description: Check if the option is used in mouse control logic and UI components. # Check mouse control logic rg -A 5 "preciseMouseInput" # Check UI components rg -A 5 "preciseMouseInput.*checkbox|toggle"Length of output: 22744
Integration of
preciseMouseInputOption: Verify UI and DocumentationThe new
preciseMouseInputoption is clearly used in the mouse control logic (see its use insrc/cameraRotationControls.ts), and it’s correctly added in the default options (src/optionsStorage.ts). However, our search did not reveal any explicit UI component (e.g., a checkbox or toggle) tied to this setting. Please confirm that one of the following is true:
- The options UI automatically renders a toggle for boolean settings like
preciseMouseInput, or- A specific UI control for this option is implemented.
Also, ensure that the documentation is updated to explain how this setting affects mouse behavior.
src/react/TouchAreasControls.tsx (3)
75-77: LGTM! Added null check and mouse state update.The null check for
botprevents potential runtime errors, and the mouse state is properly updated after the action.
87-89: LGTM! Added null check and mouse state update.The null check for
botprevents potential runtime errors, and the mouse state is properly updated after the action.
112-114: LGTM! Added null check and mouse state update.The null check for
botprevents potential runtime errors, and the mouse state is properly updated after the action.src/react/GameInteractionOverlay.tsx (1)
154-154: LGTM! Updated mouse state handling.The change aligns with the refactor to use the new mouse plugin for handling mouse state updates.
src/index.ts (1)
110-110: LGTM! Added mouse plugin integration.The mouse plugin is properly imported and loaded after bot creation, replacing the old
worldInteractionsmodule.Also applies to: 669-669
package.json (1)
149-149: New Dependency Addition – "mineflayer-mouse": "^0.0.1"The new dependency has been correctly added as per the PR objectives to enable enhanced mouse interaction support. Please verify that:
- The version "^0.0.1" meets your integration requirements.
- There are no conflicts or breaking changes introduced by this dependency with other parts of the codebase (e.g., the current mineflayer integration).
src/devtools.ts
Outdated
| window.entityCursor = () => { | ||
| return bot.mouse.getCursorState |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invoke getCursorState rather than returning the reference.
return bot.mouse.getCursorState yields the function reference, not the actual state. Use getCursorState() if you intend to retrieve the cursor information immediately.
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-ps561jtft-zaro.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/mineflayer/plugins/mouse.ts (3)
11-22: Consider using a texture atlas for destroy stage textures.Loading individual textures for each destroy stage could impact performance. Using a texture atlas would be more efficient.
Consider creating a single texture atlas containing all destroy stage textures to reduce texture switches and improve performance.
164-164: Fix TODO comment about entity attack workaround.The TODO comment indicates a workaround for entity attacks that needs to be addressed.
Would you like me to help implement a proper solution for the entity attack functionality instead of using a workaround?
188-193: Consider adding debounce to focus check.The focus check in
beforeUpdateChecksmight be called frequently. Consider adding debounce to improve performance.+let lastFocusCheck = 0 +const FOCUS_CHECK_INTERVAL = 100 // ms bot.mouse.beforeUpdateChecks = () => { + const now = Date.now() + if (now - lastFocusCheck < FOCUS_CHECK_INTERVAL) return + lastFocusCheck = now if (!document.hasFocus()) { // deactive all buttons bot.mouse.buttons.fill(false) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)src/globals.d.ts(1 hunks)src/index.ts(2 hunks)src/mineflayer/plugins/mouse.ts(1 hunks)src/react/GameInteractionOverlay.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/react/GameInteractionOverlay.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (2)
src/globals.d.ts (1)
24-24: LGTM! Improved type safety.The addition of the
voidreturn type todigStartenhances type safety and documentation.src/index.ts (1)
110-110: LGTM! Clean integration of mouse plugin.The mouse plugin is properly imported and loaded, aligning with the PR's objective of shifting mouse interaction logic to a dedicated plugin.
Also applies to: 691-691
| //@ts-expect-error | ||
| state.blockBreakMesh.material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1) | ||
| //@ts-expect-error | ||
| state.blockBreakMesh.material.needsUpdate = true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-expect-error and fix type issues.
Using @ts-expect-error suppresses type checking which could lead to runtime errors. The material property should be properly typed.
Fix the type issues by properly typing the material property:
-//@ts-expect-error
-state.blockBreakMesh.material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1)
-//@ts-expect-error
-state.blockBreakMesh.material.needsUpdate = true
+const material = state.blockBreakMesh.material as THREE.MeshBasicMaterial
+material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1)
+material.needsUpdate = true📝 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.
| //@ts-expect-error | |
| state.blockBreakMesh.material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1) | |
| //@ts-expect-error | |
| state.blockBreakMesh.material.needsUpdate = true | |
| const material = state.blockBreakMesh.material as THREE.MeshBasicMaterial | |
| material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1) | |
| material.needsUpdate = true |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-hkb72gbof-zaro.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/mineflayer/plugins/mouse.ts (2)
11-22: Consider using a texture atlas for destroy stage textures.Loading individual textures for each destroy stage increases HTTP requests and memory usage. Consider combining these textures into a single atlas texture for better performance.
186-191: Address TODO comment about entity attack handling.The current implementation uses a workaround for entity attacks. This should be replaced with a proper implementation.
Would you like me to help implement a proper entity attack handler that doesn't require the workaround?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mineflayer/plugins/mouse.ts(1 hunks)
🔇 Additional comments (3)
src/mineflayer/plugins/mouse.ts (3)
26-57: LGTM! Well-structured display manager initialization.The implementation properly initializes textures with appropriate filtering and sets up the break mesh with correct blending and render order settings.
107-110: Remove @ts-expect-error and fix type issues.Using
@ts-expect-errorsuppresses type checking which could lead to runtime errors. The material property should be properly typed.
142-149: LGTM! Clean plugin setup implementation.The plugin setup follows a logical flow, properly initializing the mouse plugin and setting up necessary listeners and display manager.
| bot.on('end', () => { | ||
| disposeObject(state.blockBreakMesh!, true) | ||
| scene.remove(state.blockBreakMesh!) | ||
| viewer.world.setHighlightCursorBlock(null) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace non-null assertions with safe null checks.
Using non-null assertions (!) could lead to runtime errors. Consider adding explicit null checks.
- disposeObject(state.blockBreakMesh!, true)
- scene.remove(state.blockBreakMesh!)
+ if (state.blockBreakMesh) {
+ disposeObject(state.blockBreakMesh, true)
+ scene.remove(state.blockBreakMesh)
+ }📝 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.
| bot.on('end', () => { | |
| disposeObject(state.blockBreakMesh!, true) | |
| scene.remove(state.blockBreakMesh!) | |
| viewer.world.setHighlightCursorBlock(null) | |
| }) | |
| bot.on('end', () => { | |
| if (state.blockBreakMesh) { | |
| disposeObject(state.blockBreakMesh, true) | |
| scene.remove(state.blockBreakMesh) | |
| } | |
| viewer.world.setHighlightCursorBlock(null) | |
| }) |
| // Update functions | ||
| function updateLineMaterial () { | ||
| const inCreative = bot.game.gameMode === 'creative' | ||
| const pixelRatio = viewer.renderer.getPixelRatio() | ||
|
|
||
| viewer.world.threejsCursorLineMaterial = new LineMaterial({ | ||
| color: (() => { | ||
| switch (options.highlightBlockColor) { | ||
| case 'blue': | ||
| return 0x40_80_ff | ||
| case 'classic': | ||
| return 0x00_00_00 | ||
| default: | ||
| return inCreative ? 0x40_80_ff : 0x00_00_00 | ||
| } | ||
| })(), | ||
| linewidth: Math.max(pixelRatio * 0.7, 1) * 2, | ||
| // dashed: true, | ||
| // dashSize: 5, | ||
| }) | ||
| } | ||
|
|
||
| function updateDisplay () { | ||
| if (viewer.world.threejsCursorLineMaterial) { | ||
| const { renderer } = viewer | ||
| viewer.world.threejsCursorLineMaterial.resolution.set(renderer.domElement.width, renderer.domElement.height) | ||
| viewer.world.threejsCursorLineMaterial.dashOffset = performance.now() / 750 | ||
| } | ||
| } | ||
| beforeRenderFrame.push(updateDisplay) | ||
|
|
||
| // Update cursor line material on game mode change | ||
| bot.on('game', updateLineMaterial) | ||
| // Update material when highlight color setting changes | ||
| subscribeKey(options, 'highlightBlockColor', updateLineMaterial) | ||
|
|
There was a problem hiding this comment.
Add missing imports for viewer and beforeRenderFrame.
The code references viewer and beforeRenderFrame which are not imported or defined in the current scope.
Add the following imports at the top of the file:
+import { viewer, beforeRenderFrame } from '../../viewer'📝 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.
| // Update functions | |
| function updateLineMaterial () { | |
| const inCreative = bot.game.gameMode === 'creative' | |
| const pixelRatio = viewer.renderer.getPixelRatio() | |
| viewer.world.threejsCursorLineMaterial = new LineMaterial({ | |
| color: (() => { | |
| switch (options.highlightBlockColor) { | |
| case 'blue': | |
| return 0x40_80_ff | |
| case 'classic': | |
| return 0x00_00_00 | |
| default: | |
| return inCreative ? 0x40_80_ff : 0x00_00_00 | |
| } | |
| })(), | |
| linewidth: Math.max(pixelRatio * 0.7, 1) * 2, | |
| // dashed: true, | |
| // dashSize: 5, | |
| }) | |
| } | |
| function updateDisplay () { | |
| if (viewer.world.threejsCursorLineMaterial) { | |
| const { renderer } = viewer | |
| viewer.world.threejsCursorLineMaterial.resolution.set(renderer.domElement.width, renderer.domElement.height) | |
| viewer.world.threejsCursorLineMaterial.dashOffset = performance.now() / 750 | |
| } | |
| } | |
| beforeRenderFrame.push(updateDisplay) | |
| // Update cursor line material on game mode change | |
| bot.on('game', updateLineMaterial) | |
| // Update material when highlight color setting changes | |
| subscribeKey(options, 'highlightBlockColor', updateLineMaterial) | |
| import { viewer, beforeRenderFrame } from '../../viewer' | |
| // Update functions | |
| function updateLineMaterial () { | |
| const inCreative = bot.game.gameMode === 'creative' | |
| const pixelRatio = viewer.renderer.getPixelRatio() | |
| viewer.world.threejsCursorLineMaterial = new LineMaterial({ | |
| color: (() => { | |
| switch (options.highlightBlockColor) { | |
| case 'blue': | |
| return 0x40_80_ff | |
| case 'classic': | |
| return 0x00_00_00 | |
| default: | |
| return inCreative ? 0x40_80_ff : 0x00_00_00 | |
| } | |
| })(), | |
| linewidth: Math.max(pixelRatio * 0.7, 1) * 2, | |
| // dashed: true, | |
| // dashSize: 5, | |
| }) | |
| } | |
| function updateDisplay () { | |
| if (viewer.world.threejsCursorLineMaterial) { | |
| const { renderer } = viewer | |
| viewer.world.threejsCursorLineMaterial.resolution.set(renderer.domElement.width, renderer.domElement.height) | |
| viewer.world.threejsCursorLineMaterial.dashOffset = performance.now() / 750 | |
| } | |
| } | |
| beforeRenderFrame.push(updateDisplay) | |
| // Update cursor line material on game mode change | |
| bot.on('game', updateLineMaterial) | |
| // Update material when highlight color setting changes | |
| subscribeKey(options, 'highlightBlockColor', updateLineMaterial) |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-p6981j2xl-zaro.vercel.app |
zardoy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/cameraRotationControls.ts (1)
31-31:⚠️ Potential issueCheck for null or undefined 'bot'.
bot.mouse.update()is called butbotisn't declared locally. Ensurebotis always initialized before this point to prevent runtime errors.Add null checks before accessing bot.mouse:
- bot.mouse.update() + if (bot?.mouse) { + bot.mouse.update() + }src/mineflayer/plugins/mouse.ts (3)
58-93:⚠️ Potential issueAdd missing imports for viewer and beforeRenderFrame.
The code references
viewerandbeforeRenderFramewhich are not imported or defined in the current scope.Add the following imports at the top of the file:
import { createMouse } from 'mineflayer-mouse' import * as THREE from 'three' import { Bot } from 'mineflayer' import { Block } from 'prismarine-block' import { Vec3 } from 'vec3' import { LineMaterial } from 'three-stdlib' import { subscribeKey } from 'valtio/utils' import { disposeObject } from 'renderer/viewer/lib/threeJsUtils' import { isGameActive, showModal } from '../../globalState' +import { viewer } from '../../globals' +import { beforeRenderFrame } from '../../index'
107-110: 🛠️ Refactor suggestionRemove @ts-expect-error and fix type issues.
Using
@ts-expect-errorsuppresses type checking which could lead to runtime errors. The material property should be properly typed.Fix the type issues by properly typing the material property:
-//@ts-expect-error -state.blockBreakMesh.material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1) -//@ts-expect-error -state.blockBreakMesh.material.needsUpdate = true +const material = state.blockBreakMesh.material as THREE.MeshBasicMaterial +material.map = state.breakTextures[stage] ?? state.breakTextures.at(-1) +material.needsUpdate = true
135-139: 🛠️ Refactor suggestionReplace non-null assertions with safe null checks.
Using non-null assertions (
!) could lead to runtime errors. Consider adding explicit null checks.- disposeObject(state.blockBreakMesh!, true) - scene.remove(state.blockBreakMesh!) + if (state.blockBreakMesh) { + disposeObject(state.blockBreakMesh, true) + scene.remove(state.blockBreakMesh) + }
🧹 Nitpick comments (2)
src/mineflayer/plugins/mouse.ts (2)
40-45: Add texture disposal to prevent memory leaks.The textures loaded for break animations are never disposed, which could lead to memory leaks when the plugin is unloaded.
Add texture disposal in the bot's end event handler:
bot.on('end', () => { if (state.blockBreakMesh) { disposeObject(state.blockBreakMesh, true) scene.remove(state.blockBreakMesh) } viewer.world.setHighlightCursorBlock(null) + // Dispose textures to prevent memory leaks + state.breakTextures.forEach(texture => { + texture.dispose() + }) + state.breakTextures = [] })
142-149: Add proper module exports and type interfaces.The mouse plugin implementation would benefit from clearer exports and interfaces for better code organization.
Consider extracting the display manager and listeners into named exports with proper interfaces:
interface MouseDisplayManager { updateBreakAnimation: (block?: Block, stage?: number | null) => void hideBreakAnimation: () => void updateCursorBlock: (data?: { block: Block }) => void dispose: () => void } export function createMouseDisplayManager(bot: Bot, scene: THREE.Scene, renderer: THREE.WebGLRenderer): MouseDisplayManager { // Implementation } export default function(bot: Bot) { // Plugin setup }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)src/cameraRotationControls.ts(1 hunks)src/index.ts(2 hunks)src/mineflayer/plugins/mouse.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (3)
src/index.ts (2)
110-110: Clean and appropriate import added.The mouse plugin import is added correctly, which aligns with the PR objective of refactoring mouse controls.
692-692: Plugin loading looks good.The mouse plugin is properly loaded into the bot instance, replacing the previous worldInteractions functionality.
src/cameraRotationControls.ts (1)
23-23: Improved mouse input control with new option.Adding the
preciseMouseInputcondition provides more granular control over mouse sensitivity and movement.
PR Type
Enhancement, Bug fix
Description
Refactored sound system to improve sound handling.
soundsIdToNamemapping for better sound identification.Enhanced block sound alias mappings for better categorization.
Removed redundant or unused code for optimization.
Commented out unused
soundsbundling inmakeOptimizedMcData.mjs.Changes walkthrough 📝
botSoundSystem.ts
Refactored sound handling logic in bot sound systemsrc/sounds/botSoundSystem.ts
soundsMap.ts
Improved sound mapping and block sound alias handlingsrc/sounds/soundsMap.ts
soundsIdToNamemapping for sound identification.makeOptimizedMcData.mjs
Removed unused sounds bundling configurationscripts/makeOptimizedMcData.mjs
soundsbundling configuration.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
worldInteractionsmodule to focus on themouseplugin.