fix: fix some window titles #401
Conversation
WalkthroughThe update imports the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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 (1)
src/inventoryWindows.ts (1)
390-398: Consider enhancing error handling for title formatting.While the try-catch block prevents crashes, the error is only reported via
reportError?.(err)and falls back toundefined. Consider logging more specific information about title formatting failures for debugging purposes.} catch (err) { - reportError?.(err) + console.warn('Failed to format window title:', { title, error: err }) + reportError?.(err) inv.canvasManager.children[0].customTitleText = undefined }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/inventoryWindows.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zardoy
PR: zardoy/minecraft-web-client#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.
src/inventoryWindows.ts (5)
Learnt from: zardoy
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
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
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).
Learnt from: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
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).
Learnt from: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
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.
Learnt from: zardoy
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.
🔇 Additional comments (4)
src/inventoryWindows.ts (4)
12-12: LGTM: Import addition looks correct.The
prismarine-nbtimport is appropriately added to handle NBT data simplification for window titles.
358-358: Function signature update is correctly implemented.The
openWindowfunction signature now properly accepts an optionaltitleparameter with a default value ofundefined. This maintains backward compatibility while enabling the new title functionality.
390-394: Title handling logic correctly uses the passed parameter.The implementation properly uses the passed
titleparameter instead of retrieving frombot.currentWindow?.title. The conditional logic handles both string and object titles appropriately, with proper error handling via try-catch.
63-63: Verify NBT simplification in window titlesThe use of
nbt.simplify(win.title as any)here:
- Loses compile-time guarantees by casting to
any. Consider replacing with a more specific type (e.g. the Prismarine-styleTagor a custom ChatComponent interface).- May strip out nested chat component properties (color, bold, italics, hover events, etc.). Ensure that your window titles’ formatting fields survive the simplification step.
- Add a quick smoke-test: open a GUI with a colored/styled title and confirm it renders correctly.
Locations to check:
src/inventoryWindows.ts:63src/inventoryWindows.ts:65
2 participants
Fixes the issue of chest names (and similar containers) weren't shown.
Still missing:
Summary by CodeRabbit