fix: Effects and Game Indicators overlay toggles didn't work #397
Conversation
WalkthroughThe component in Changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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/react/IndicatorEffectsProvider.tsx (1)
123-124: Consider removing redundant prop passing.Since the conditional logic is now handled in the parent component (lines 121-122), passing
displayIndicatorsanddisplayEffectsas props to the child component might be redundant. The child component should no longer need these boolean flags since the data is already filtered based on their values.Consider removing these props if they're no longer used in the
IndicatorEffectscomponent:return <IndicatorEffects indicators={displayIndicators ? allIndicators : defaultIndicatorsState} effects={displayEffects ? effects : []} - displayIndicators - displayEffects />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/IndicatorEffectsProvider.tsx(1 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.
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, 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.
src/react/IndicatorEffectsProvider.tsx (3)
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: CR
PR: zardoy/minecraft-web-client#0
File: .cursor/rules/vars-usage.mdc:0-0
Timestamp: 2025-07-02T02:50:46.443Z
Learning: Some other global variables that can be used without window prefixes are listed in src/globals.d.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, all bot/player state and events must be accessed via explicit interfaces, state managers, or passed-in objects, never by referencing `bot` directly.
🧬 Code Graph Analysis (1)
src/react/IndicatorEffectsProvider.tsx (1)
src/react/IndicatorEffects.tsx (1)
defaultIndicatorsState(63-71)
🔇 Additional comments (1)
src/react/IndicatorEffectsProvider.tsx (1)
121-122: LGTM! The conditional logic correctly implements the toggle functionality.The changes properly address the issue where overlay toggles weren't working. The conditional logic ensures that when
displayIndicatorsis false, the component receivesdefaultIndicatorsState(effectively hiding indicators), and whendisplayEffectsis false, it receives an empty array (hiding effects).
Hm, I didn't even notice this. It's weird that this doesn't work. Looking into that. |
|
Apparently it's required to explicitly set the values there which is why it didn't work originally. (They were always treated as |
|
yeah, another silly refactor. TYSM |
2 participants
This fixes that disabling the effects and game indicators overlays wasn't working via the config gui or
disabledUiPartsquery parameters as the passed boolean parameters were never used in theIndicatorEffectsProvider.tsxSummary by CodeRabbit