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

Rewrite of the colour skill & refactoring of difficulty calculation within osu!taiko #19571

Merged
merged 75 commits into from
Aug 25, 2022

Conversation

vunyunt
Copy link
Contributor

@vunyunt vunyunt commented Aug 3, 2022

Reopen of #19184 and #19566

Apologies for the clutter, I have to reopen this from my personal repository due to the Allow edits and access to secrets by maintainers option not being available to me or Lawtrohux when opening from the taiko-pp-committee repository.


This PR aims to get all the major refactoring out of the way, and with the addition of the rewritten colour skill. Apologies for the large size of the PR, as everything is much too intertwined to properly split this PR. This should be the last PR of its out of proportion scale.

Full rewrite on the colour skill based on information density. See here for a more detailed explanation.

  • Adjusted weight of each skill. Rhythm is intentionally weighted lower due to it not being worked on yet.
  • Changes to taiko difficulty hit object to incorporate evaluators.
  • Ported stamina into evaluator.
  • Changed stamina speed bonus to be 1 / interval based.
  • Removed separated ratings for final SR Calculation.
  • Moved locally combined difficulty (strain peaks) into the peaks skill, all other skills are now under this.
  • Changed locally combined difficulty to use 2-norm to combine stamina and colour, and a 1.5-norm to combine that with rhythm.
  • Addition of data directories, which will be used in future references per skill.
  • Addition of per skill preprocessing, to clean up clutter.

SR/PP sheet versus master prior to #19181 : https://docs.google.com/spreadsheets/d/1V27Jiyg0sD7zQuVoklkbW10fPpIMVqXEkblZkolyDD8/edit

As of fb9bb2d

vunyunt and others added 30 commits May 24, 2022 17:38
I read through this thinking "why doesn't Previous get assigned to
currentEncoding here? But it's because the initializer runs right after
the ctor and before the "method" returns. So really there's 3 operations
running on one line here - ctor, init, and assignment.
smoogipoo
smoogipoo previously approved these changes Aug 15, 2022
@smoogipoo
Copy link
Contributor

I'm doing one last sheet generation to make sure I didn't break anything here. The taiko committee previously had approvals on this and the code looks fine to me, without diving into the maths.

Can I get a quick second look over this from one of you @peppy @bdach ? I intend to get this out very soon.

@bdach bdach self-requested a review August 15, 2022 13:58
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

I skimmed this ignoring essentially all of the math, focus was on naming and structure. Which was really difficult to follow as far as I'm concerned.

@bdach
Copy link
Collaborator

bdach commented Aug 23, 2022

With the latest changes the diff looks much better to me. Save for one unaddressed review comment this looks pretty much ready as far as I'm concerned.

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

No more objections here. Fine to proceed as far as I'm concerned.

@smoogipoo smoogipoo merged commit 136dcee into ppy:master Aug 25, 2022
@vunyunt vunyunt deleted the colour-encoding-2 branch January 22, 2024 00:01
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.

5 participants