Skip to content

fix: Improve progress calculation and reduce update interval in BossB… #110

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: Improve progress calculation and reduce update interval in BossB… #110

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f705236
fix: Improve progress calculation and reduce update interval in BossB…
ChudziudgiToJa Oct 17, 2025
66d25ea
fix: Add configurable update interval for progress updates in BossBar…
ChudziudgiToJa Oct 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ...cation-core/src/com/eternalcode/multification/notice/resolver/bossbar/BossBarService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ private void updateProgress(Instant expiration, Duration duration, BossBar bossB
}

Duration remaining = Duration.between(Instant.now(), expiration);
float progress = 1 - (float) remaining.getSeconds() / duration.getSeconds();
double totalMillis = duration.toMillis();
double remainingMillis = Math.max(0, remaining.toMillis());
float progress = (float) (remainingMillis / totalMillis);
Copy link

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current progress calculation might lead to unexpected behavior due to integer division. Specifically, remainingMillis / totalMillis could result in 0 if remainingMillis is less than totalMillis but still greater than 0, causing the boss bar to appear empty prematurely. To fix this, ensure that the division is performed with double precision before casting to float.

Consider swapping the order of the cast and the division to ensure floating point division is performed.

Suggested change
double totalMillis = duration.toMillis();
double remainingMillis = Math.max(0, remaining.toMillis());
float progress = (float) (remainingMillis / totalMillis);
double totalMillis = duration.toMillis();
double remainingMillis = Math.max(0, remaining.toMillis());
float progress = (float) remainingMillis / (float) totalMillis; // Explicitly cast both operands to float before division

progress = Math.max(0.0f, Math.min(1.0f, progress));
Copy link

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The progress value is clamped between 0.0 and 1.0, which is good for preventing out-of-bounds values. However, the logic is flawed. The progress should represent the percentage of time that has elapsed, not the time remaining. The current calculation shows the remaining time. This should be inverted to represent elapsed time.

Consider inverting the progress calculation to reflect elapsed time instead of remaining time.

Suggested change
float progress = (float) (remainingMillis / totalMillis);
progress = Math.max(0.0f, Math.min(1.0f, progress));
double totalMillis = duration.toMillis();
double remainingMillis = Math.max(0, remaining.toMillis());
float progress = (float) (1.0 - (remainingMillis / totalMillis)); // Invert the progress calculation
progress = Math.max(0.0f, Math.min(1.0f, progress));


bossBar.progress(progress);

this.scheduler.schedule(() -> updateProgress(expiration, duration, bossBar, viewer), 500L, TimeUnit.MILLISECONDS);
this.scheduler.schedule(() -> updateProgress(expiration, duration, bossBar, viewer), 100L, TimeUnit.MILLISECONDS);
Copy link

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Reducing the scheduler interval to 100ms can lead to increased CPU usage, especially if there are many boss bars being updated simultaneously. While this provides a smoother visual update, it's important to consider the performance implications. Make sure this value is configurable or that performance is carefully monitored.

Consider making the update interval configurable to allow administrators to tune performance.

        long updateInterval = 100L; // Make this configurable
        this.scheduler.schedule(() -> updateProgress(expiration, duration, bossBar, viewer), updateInterval, TimeUnit.MILLISECONDS);

}

}