Add OsuHealthProcessor that uses the legacy drain rate algorithm
#25418
Conversation
| currentHpUncapped = hp_bar_maximum; | ||
|
|
||
| double lowestHp = currentHp; | ||
| double lastTime = DrainStartTime; |
There was a problem hiding this comment.
Just gonna note that this isn't exactly the same as stable, since DrainStartTime comes from DrainingHealthProcessor ctor param, which comes from OsuHealthProcessor ctor param, which comes from OsuRuleset.CreateHealthProcessor() ctor param, which is given as playableBeatmap.HitObjects[0].StartTime:
https://github.com/ppy/osu/blob/master/osu.Game/Screens/Play/Player.cs#L230
while stable gives it hitObjectManager.hitObjects[0].StartTime - hitObjectManager.PreEmpt (https://github.com/peppy/osu-stable-reference/blob/master/osu!/GameModes/Play/Rulesets/Osu/RulesetOsu_HpCalculation.cs).
Likely not a huge deal in practice, though.
There was a problem hiding this comment.
@smoogipoo can you confirm you're okay with this?
There was a problem hiding this comment.
Yes, the value is a best effort but is also accurate to how lazer works - health only starts draining from that time.
|
Checking with stable, the legacy health processor values look to be matching stable log output on a few random maps from the sheet, so the ground truth appears to be correct there. |
| /// <summary> | ||
| /// The drain rate as a proportion of the total health drained per millisecond. | ||
| /// </summary> | ||
| public double DrainRate { get; private set; } = 1; |
There was a problem hiding this comment.
I know this is coming from the old code, but how about we get rid of this weird default value? It would mean the whole health would drain in 1 millisecond, which makes little sense.
diff --git a/osu.Game/Rulesets/Scoring/DrainingHealthProcessor.cs b/osu.Game/Rulesets/Scoring/DrainingHealthProcessor.cs
index a8808d08e5..4a651e2650 100644
--- a/osu.Game/Rulesets/Scoring/DrainingHealthProcessor.cs
+++ b/osu.Game/Rulesets/Scoring/DrainingHealthProcessor.cs
@@ -44,7 +44,7 @@ public partial class DrainingHealthProcessor : HealthProcessor
/// <summary>
/// The drain rate as a proportion of the total health drained per millisecond.
/// </summary>
- public double DrainRate { get; private set; } = 1;
+ public double DrainRate { get; private set; }
/// <summary>
/// The beatmap.
@@ -139,8 +139,6 @@ protected override void Reset(bool storeResults)
{
base.Reset(storeResults);
- DrainRate = 1;
-
if (storeResults)
DrainRate = ComputeDrainRate();
|
|
||
| // TODO: This doesn't handle overlapping/sequential breaks correctly (/b/614). | ||
| // Subtract any break time from the duration since the last object | ||
| if (Beatmap.Breaks.Count > 0 && currentBreak < Beatmap.Breaks.Count) |
There was a problem hiding this comment.
This logic is shockingly written, but I'm not going to touch it as it does match the stable code.
There was a problem hiding this comment.
Intended to resolve #24365
I've tried to get the existing lazer algorithm to match more closely to osu!stable but the best I can do is +/- 40% difference in drain rate. I was told this wasn't acceptable, so here we are.
This PR adds two health processors:
LegacyOsuHealthProcessorwhich implements the legacy algorithm exactly, andOsuHealthProcessorwhich is lazer's take on it.Results can be found here: https://docs.google.com/spreadsheets/d/1npZHxhV-ZtQxDQ1KM2OBAXBcwhrfl-8ZWccq7xqQ-g8/edit#gid=2107058059
Drain rate exactly matches osu!stable if the combo end bonus is not added in. This is an osu!stable-specific gameplay mechanic which we have not yet implemented in lazer (geki bonus, for successfully completing a combo). We've briefly discussed this and may consider implementing it in lazer in the future - we had earlier intentions of doing so at least.
There are bugs in the implementation - breaks are not handled correctly by the legacy algorithm, and there are also edge cases around sliders/spinners. These are taken verbatim from osu!stable without having been fixed, but have been noted in the implementation.
There's an osu-tools branch with a new command that can be used to test this. I haven't PR'd it because it's a very specific use-case of testing out this one PR.
I haven't done any work on other rulesets yet.