Skip to content

Refactor hit result methods on Ruleset#36639

Merged
peppy merged 3 commits intoppy:masterfrom
bdach:actually-all-valid-hit-results
Feb 10, 2026
Merged

Refactor hit result methods on Ruleset#36639
peppy merged 3 commits intoppy:masterfrom
bdach:actually-all-valid-hit-results

Conversation

@bdach
Copy link
Copy Markdown
Collaborator

@bdach bdach commented Feb 9, 2026

  • GetHitResults() gets a ForDisplay suffix to the name to indicate its purpose better.
  • GetValidHitResults() becomes public and actually returns all valid hit results rather than ignoring the existence of miss types and technical result types like IgnoreHit or ComboBreak.

Notably, of the two aforementioned methods, the latter is only called by the former. All changes to the latter do not have any impact on the former; this was mostly ensured by GetHitResultsDisplay() ignoring all non-full-miss types and IgnoreHit, I just had to tack ComboBreak onto that list.

With the structure of everything in hit objects this is kinda horrifyingly difficult to verify the correctness of. I'm only like 90% sure myself. This is because hit results are declared by judgements which are used by hit objects, except judgements only declare maximum hit results, and minimum hit results are implicitly derived from maximum results in Judgement, unless that Judgement property is overridden, and so on and so forth. So I think the baseline bonus - except being able to use the new public method for actual validation - is at least in cleartext documentation of what results a ruleset actually uses.

@bdach bdach requested a review from peppy February 9, 2026 11:50
@bdach bdach self-assigned this Feb 9, 2026
Copy link
Copy Markdown
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

I think this is fine..

@peppy peppy merged commit c59e5bf into ppy:master Feb 10, 2026
10 checks passed
@bdach bdach deleted the actually-all-valid-hit-results branch February 10, 2026 08:18
bdach added a commit to bdach/osu-queue-score-statistics that referenced this pull request Feb 10, 2026
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.

2 participants