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

Fix Strict Tracking mod sliderend combo issue #19271

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

Conversation

Cwazywierdo
Copy link
Contributor

Closes #19232

Co-authored-by: Bartłomiej Dach <[email protected]>
@bdach
Copy link
Collaborator

bdach commented Jul 22, 2022

In theory this looks better now, but there's a display issue. If slider tails are to be judged as SliderTick, then this becomes a problem:

public override string GetDisplayNameForHitResult(HitResult result)
{
switch (result)
{
case HitResult.LargeTickHit:
return "slider tick";
case HitResult.SmallTickHit:
return "slider end";
case HitResult.SmallBonus:
return "spinner spin";
case HitResult.LargeBonus:
return "spinner bonus";
}
return base.GetDisplayNameForHitResult(result);
}

The fact that the judgement names are overridden makes this confusing because it will seem like tails have been "replaced by ticks". Related: #18764 (comment)

@frenzibyte
Copy link
Member

The only simplest way I can think of is allowing Mods to provide a map between the original HitResult and the modified version, and using that in GetStatisticsForDisplay. That sounds super dodge though, and feels like HitResult isn't a good enum to use for mapping to judgement names (perhaps a Judgement.Name property would be better, along with a list of valid Judgements provided at Ruleset or otherwise).

@bdach
Copy link
Collaborator

bdach commented Jul 23, 2022

The only simplest way I can think of is allowing Mods to provide a map between the original HitResult and the modified version, and using that in GetStatisticsForDisplay.

No, the simplest thing to do here - code-wise - would be to remove GetDisplayNameForHitResult() entirely. Which is the one I'd probably vote for, on technical grounds alone. There are already two known cases of this method not really going well together with intentions in that respect. Unfortunately this would be done at the expense of user understanding.

@frenzibyte
Copy link
Member

frenzibyte commented Jul 24, 2022

Yeah actually, upon taking a second look this doesn't seem feasible to fix without making each mod specify which HitResults they changed, and that doesn't look pleasant to have (unless we fire up a ScoreProcessor for every score and map judgement results to the hit results in the statistics). But on the other hand, I'm quite unsure about displaying "large tick" and "small tick" to the user, and have them mentally map that to the slider tick/end.

@peppy peppy marked this pull request as draft June 16, 2023 06:09
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.

Strict Tracking causes leaderboard ordering to be wrong with Classic scoring
4 participants