Skip to content
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

Add Gameplay Accuracy Heatmap #31158

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

Conversation

normalid-awa
Copy link
Contributor

It is the accuracy heatmap but in real-time.

Available in HUD(osu only) and Playfield(osu only) layer

image

osu.2024.12.16.-.22.12.44.08.mp4

@Joehuu
Copy link
Member

Joehuu commented Dec 16, 2024

There's already #25716, but judging the lower LOC here this seems better as it's not reimplementing everything (e.g. the design).

The linked PR does have early-late color indicators though, so not sure.

@normalid-awa
Copy link
Contributor Author

normalid-awa commented Dec 17, 2024

There's already #25716, but judging the lower LOC here this seems better as it's not reimplementing everything (e.g. the design).

The linked PR does have early-late color indicators though, so not sure.

Well, actually the next step action I would take is to include the late-early click indicator color to accuracy heatmap

Comment on lines +48 to +51
if (gameplayClockContainer != null)
gameplayClockContainer.OnSeek += initHeatmap;

scoreProcessor.NewJudgement += updateHeatmap;
Copy link
Collaborator

Choose a reason for hiding this comment

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

drawable adding event subscriptions to other components that are not cleaned up on disposal => this will leak delegates and hold references to removed instances of this component forever

{
if (pointGrid.Content.Count == 0)
if (pointGrid == null || pointGrid.Content.Count == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is with the null check here? sounds to me like you're calling AddPoint() too early before the pointGrid has been created and trying to paper over that crack by doing this. this is not the correct way to do this, you must somehow make sure that the calls to AddPoint() that occur too early take place when everything else is ready. many ways to do this, including but not limited to scheduling or queueing the calls in a list until loaded

if (j is not OsuHitCircleJudgementResult circleJudgementResult || circleJudgementResult.CursorPositionAtHit == null)
return;

heatmap.AddPoint(((OsuHitObject)j.HitObject).StackedEndPosition, ((OsuHitObject)j.HitObject).StackedEndPosition, circleJudgementResult.CursorPositionAtHit.Value, radius);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this using CursorPositionAtHit rather than plain Position like the other existing usage of this?

AddPoint(((OsuHitObject)e.LastHitObject).StackedEndPosition, ((OsuHitObject)e.HitObject).StackedEndPosition, e.Position.Value, radius);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants