feat: enable bossbar by default & support for custom title texts #259

Phoenix616 · 2025-01-31T15:14:58Z

User description

This fixes that the bossbar would not be rendered properly when added via the /bossbar command/plugin API and wasn't using the component rendering system but manual mapping. Also updates to the boss bar would not get rendered before.

PR Type

Bug fix, Enhancement

Description

Fixed boss bar titles not displaying correctly.
Updated boss bar title handling to support various formats.
Refactored boss bar management with dedicated add/remove functions.
Improved boss bar update logic for better rendering.

Changes walkthrough 📝

Relevant files

Enhancement

BossBarOverlay.tsx `Refactor boss bar title handling and styles` src/react/BossBarOverlay.tsx Changed boss bar title handling to support multiple formats. Removed manual translation mapping for titles. Updated state initialization for boss bar title. Adjusted logic for setting boss bar styles.	+4/-8
BossBarOverlayProvider.tsx `Refactor boss bar management with utility functions` src/react/BossBarOverlayProvider.tsx Added `addBossBar` and `removeBossBar` utility functions. Simplified boss bar event handling with new functions. Improved boss bar update logic with a timeout mechanism.	+15/-5

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

codesandbox · 2025-01-31T15:15:02Z

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

qodo-merge-pro-for-open-source · 2025-01-31T15:15:19Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Race Condition The setTimeout with 1ms delay in bossBarUpdated event handler could lead to race conditions. Consider using React state updates properly instead of this workaround. setTimeout(() => addBossBar(bossBar as BossBarType), 1) Type Safety The title type allows any Record<string,any> which is too permissive and could lead to runtime errors. Consider using a more specific type definition. title: string \| Record<string, any> \| null, _title: string \| Record<string, any> \| null,

qodo-merge-pro-for-open-source · 2025-01-31T15:15:36Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Remove unnecessary state update delay Remove the unnecessary timeout when updating boss bars. The `removeBossBar` followed by a delayed `addBossBar` creates a visual flicker and race condition risk. Instead, directly update the boss bar state. src/react/BossBarOverlayProvider.tsx [24-27] bot.on('bossBarUpdated', (bossBar) => { - removeBossBar(bossBar as BossBarType) - setTimeout(() => addBossBar(bossBar as BossBarType), 1) + addBossBar(bossBar as BossBarType) }) Apply this suggestion Suggestion importance[1-10]: 8 Why: The current implementation using setTimeout creates unnecessary visual flicker and potential race conditions. Direct state update is cleaner and more reliable.	8
General	Add proper type safety Initialize the title state with a proper type annotation matching the possible title formats (string \| Record<string, any> \| null) to prevent type errors. src/react/BossBarOverlay.tsx [19] -const [title, setTitle] = useState({}) +const [title, setTitle] = useState<string \| Record<string, any> \| null>(null) Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion correctly aligns the useState type with the BossBarType interface definition, preventing potential type-related bugs and improving code maintainability.	7

zardoy · 2025-01-31T15:27:40Z

/deploy

github-actions · 2025-01-31T15:29:37Z

Deployed to Vercel Preview: https://prismarine-p2emo882d-zaro.vercel.app
Playground
Storybook

zardoy

cant get it working, @Phoenix616 please specify versions on which you test your fixes

zardoy · 2025-01-31T15:50:43Z

src/react/BossBarOverlayProvider.tsx

    })
    bot.on('bossBarUpdated', (bossBar) => {
-      setBossBars(prevBossBars => new Map(prevBossBars.set(bossBar.entityUUID, bossBar as BossBarType)))
+      removeBossBar(bossBar as BossBarType)
+      setTimeout(() => addBossBar(bossBar as BossBarType), 1)


why do we need timeout? would not create it flickering, so weird

It will otherwise not update the displayed text/values/styles, most likely because the element already exists? It was the only solution I found but I don't know how react works so there might be a better way to force an update of the changed element.

zardoy · 2025-01-31T16:07:47Z

ok, my apologies, it is not visible only after rejoining.

though IMO further polishing is possible (wdyt?):
current:

vanilla:
mobile current:

IMO need to:

bossbar-title class: set font size to 9px (make a bit bigger), add margin-bottom to at least 1px (can leave as 1px for now)
when touch active: .bossBars top = 18px , when not 1px
add pointerEvents none to bossBars
lets enable them by default

Phoenix616 · 2025-01-31T16:19:17Z

Style changes make sense, yeah.

I'm unsure what you mean by "lets enable them by default" though. I didn't adjust any code regarding the behavior or when and how it displays, it instantly shows them as soon as the mineflayer bot event is called which should be whenever a packet is received.

zardoy · 2025-01-31T19:53:34Z

Style changes make sense, yeah.

I'm unsure what you mean by "lets enable them by default" though. I didn't adjust any code regarding the behavior or when and how it displays, it instantly shows them as soon as the mineflayer bot event is called which should be whenever a packet is received.

no worries I can these changes myself, I just have hundreds of tasks that I try to do at the same and had to write these here so I don't forget

@Phoenix616 most importantly I wanted to know your opinion on these changes and if you have any other feedback

- Adjusts the font size and gabs - Fixes that it would overlap the mobile buttons - Fixes that touching where the boss bar is would not move the camera - Better match the behaviour of really wide bossbar titles

Phoenix616 · 2025-02-03T15:21:06Z

Did some adjustments to the bossbar styles to make them behave pretty close to Java (imo) and also fixed the mobile issues at the same time.

github-actions · 2025-02-03T15:21:46Z

Deployed to Vercel Preview: https://prismarine-3lkbca6zf-zaro.vercel.app
Playground
Storybook

zardoy · 2025-02-04T04:59:49Z

@Phoenix616 again, i still do not understand the intention to for setTimeout, will ltry to figure out as well will try to fix the re-login issue

zardoy · 2025-02-04T05:07:08Z

ok dont have time, feel free to help, need a plugin in src/mineflayer/plugins/bossbars.ts so bossbars are preserved into mineflayer bot state

github-actions · 2025-02-04T05:07:24Z

Deployed to Vercel Preview: https://prismarine-9w6g5l9b1-zaro.vercel.app
Playground
Storybook

fix: bossbar does not display custom titles

ca6a092

qodo-merge-pro-for-open-source bot added the Review effort [1-5]: 2 label Jan 31, 2025

zardoy requested changes Jan 31, 2025

View reviewed changes

fix: style of the boss bars not matching and mobile issues

0f60b5e

- Adjusts the font size and gabs - Fixes that it would overlap the mobile buttons - Fixes that touching where the boss bar is would not move the camera - Better match the behaviour of really wide bossbar titles

enable bossbars

216c2ed

fix mobile class

71960e2

zardoy changed the title ~~fix: bossbar does not display custom titles~~ feat: enable bossbar by default & support for custom title texts Feb 4, 2025

zardoy merged commit e2a093b into zardoy:next Feb 4, 2025
0 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable bossbar by default & support for custom title texts #259

feat: enable bossbar by default & support for custom title texts #259

Phoenix616 commented Jan 31, 2025 •

edited by qodo-merge-pro-for-open-source bot

Loading

codesandbox bot commented Jan 31, 2025

qodo-merge-pro-for-open-source bot commented Jan 31, 2025

qodo-merge-pro-for-open-source bot commented Jan 31, 2025

zardoy commented Jan 31, 2025

github-actions bot commented Jan 31, 2025

zardoy left a comment •

edited

Loading

zardoy Jan 31, 2025

Phoenix616 Jan 31, 2025 •

edited

Loading

zardoy commented Jan 31, 2025 •

edited

Loading

Phoenix616 commented Jan 31, 2025

zardoy commented Jan 31, 2025 •

edited

Loading

Phoenix616 commented Feb 3, 2025 •

edited

Loading

github-actions bot commented Feb 3, 2025

zardoy commented Feb 4, 2025

zardoy commented Feb 4, 2025

github-actions bot commented Feb 4, 2025

feat: enable bossbar by default & support for custom title texts #259

feat: enable bossbar by default & support for custom title texts #259

Conversation

Phoenix616 commented Jan 31, 2025 • edited by qodo-merge-pro-for-open-source bot Loading

User description

PR Type

Description

Changes walkthrough 📝

codesandbox bot commented Jan 31, 2025

Review or Edit in CodeSandbox

qodo-merge-pro-for-open-source bot commented Jan 31, 2025

PR Reviewer Guide 🔍

qodo-merge-pro-for-open-source bot commented Jan 31, 2025

PR Code Suggestions ✨

zardoy commented Jan 31, 2025

github-actions bot commented Jan 31, 2025

zardoy left a comment • edited Loading

Choose a reason for hiding this comment

zardoy Jan 31, 2025

Choose a reason for hiding this comment

Phoenix616 Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

zardoy commented Jan 31, 2025 • edited Loading

Phoenix616 commented Jan 31, 2025

zardoy commented Jan 31, 2025 • edited Loading

Phoenix616 commented Feb 3, 2025 • edited Loading

github-actions bot commented Feb 3, 2025

zardoy commented Feb 4, 2025

zardoy commented Feb 4, 2025

github-actions bot commented Feb 4, 2025

Phoenix616 commented Jan 31, 2025 •

edited by qodo-merge-pro-for-open-source bot

Loading

zardoy left a comment •

edited

Loading

Phoenix616 Jan 31, 2025 •

edited

Loading

zardoy commented Jan 31, 2025 •

edited

Loading

zardoy commented Jan 31, 2025 •

edited

Loading

Phoenix616 commented Feb 3, 2025 •

edited

Loading