-
Notifications
You must be signed in to change notification settings - Fork 114
fix: adding support for newer skin profile data structure in player heads #424
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
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe head rendering now reads base64 texture data from either Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant R as WorldRendererThree
participant D as Skull/Profile Data
participant B as Base64 Decoder
participant P as Proxy Rewriter
participant T as Texture Loader
R->>D: Read SkullOwner.Properties.textures[0].Value
alt SkullOwner textures present
D-->>R: base64 textureData
else Fallback to profile.properties
R->>D: Find property name === "textures"
D-->>R: base64 textureData (fallback)
end
alt textureData missing
R-->>R: Early return (no head texture)
else
R->>B: Decode Buffer.from(textureData, 'base64')
B-->>R: decodedData (JSON)
R->>R: Extract decodedData.textures.SKIN.url
R->>P: Rewrite URL via skinTexturesProxy (http/https)
P-->>R: Proxied URL
R->>T: Load texture and build head mesh
T-->>R: Mesh/Texture applied
end
note over R: Errors are caught and logged
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ 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 (
|
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
renderer/viewer/three/worldrendererThree.ts (1)
779-785
: Guard against missing skinUrl and fix replace chaining (can crash on undefined).When
decodedData.textures?.SKIN?.url
is absent,skinUrl?.replace(...).replace(...)
evaluates the second.replace
onundefined
. Add an early return and drop optional chaining in replacements.- let skinUrl = decodedData.textures?.SKIN?.url - const { skinTexturesProxy } = this.worldRendererConfig - if (skinTexturesProxy) { - skinUrl = skinUrl?.replace('http://textures.minecraft.net/', skinTexturesProxy) - .replace('https://textures.minecraft.net/', skinTexturesProxy) - } + let skinUrl = decodedData.textures?.SKIN?.url + if (!skinUrl) return + const { skinTexturesProxy } = this.worldRendererConfig + if (skinTexturesProxy) { + skinUrl = skinUrl + .replace('http://textures.minecraft.net/', skinTexturesProxy) + .replace('https://textures.minecraft.net/', skinTexturesProxy) + }
📜 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/worldrendererThree.ts
(1 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 variablebot
directly 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 referencingbot
directly.
In renderer code (such as inWorldRendererThree
), use theplayerState
property (e.g.,worldRenderer.playerState.gameMode
) to access player state. The implementation forplayerState
lives insrc/mineflayer/playerState.ts
.
Files:
renderer/viewer/three/worldrendererThree.ts
🧠 Learnings (2)
📚 Learning: 2025-06-23T13:33:14.776Z
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.
Applied to files:
renderer/viewer/three/worldrendererThree.ts
📚 Learning: 2025-07-27T13:24:58.186Z
Learnt from: CR
PR: zardoy/minecraft-web-client#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:
renderer/viewer/three/worldrendererThree.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
When a player head has a
data.profile.properties
rather thandata.SkullOwner.Properties
data structure, it weren't rendered with its skin texture.This fixes it by implementing a check wheather
SkullOwner
exists. This was also done in a similar way here:minecraft-web-client/renderer/viewer/three/entities.ts
Lines 1407 to 1412 in 9d54c70
Summary by CodeRabbit