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

Block input to objects lying under already-hit hitcircles when classic note lock is active #24720

Merged
merged 3 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleLateFade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ private TestDrawableHitCircle createCircle(bool shouldHit = false)

drawableHitCircle.Scale = new Vector2(2f);

LoadComponent(drawableHitCircle);
foreach (var mod in SelectedMods.Value.OfType<IApplicableToDrawableHitObject>())
mod.ApplyToDrawableHitObject(drawableHitCircle);

Expand Down
56 changes: 56 additions & 0 deletions osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,56 @@ public void TestAutopilotReducesHittableRange()
addClickActionAssert(0, ClickAction.Shake);
}

[Test]
public void TestInputDoesNotFallThroughOverlappingSliders()
{
const double time_first_slider = 1000;
const double time_second_slider = 1250;
Vector2 positionFirstSlider = new Vector2(100, 50);
Vector2 positionSecondSlider = new Vector2(100, 80);
var midpoint = (positionFirstSlider + positionSecondSlider) / 2;

var hitObjects = new List<OsuHitObject>
{
new Slider
{
StartTime = time_first_slider,
Position = positionFirstSlider,
Path = new SliderPath(PathType.Linear, new[]
{
Vector2.Zero,
new Vector2(25, 0),
})
},
new Slider
{
StartTime = time_second_slider,
Position = positionSecondSlider,
Path = new SliderPath(PathType.Linear, new[]
{
Vector2.Zero,
new Vector2(25, 0),
})
}
};

performTest(hitObjects, new List<ReplayFrame>
{
new OsuReplayFrame { Time = time_first_slider, Position = midpoint, Actions = { OsuAction.RightButton } },
new OsuReplayFrame { Time = time_first_slider + 25, Position = midpoint, Actions = { OsuAction.LeftButton } },
new OsuReplayFrame { Time = time_first_slider + 50, Position = midpoint },
});

addJudgementAssert(hitObjects[0], HitResult.Ok);
addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0);
addJudgementAssert(hitObjects[1], HitResult.Miss);
// the slider head of the first slider prevents the second slider's head from being hit, so the judgement offset should be very late.
// this is not strictly done by the hit policy implementation itself (see `OsuModClassic.blockInputToUnderlyingObjects()`),
// but we're testing this here anyways to just keep everything related to input handling and note lock in one place.
addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, referenceHitWindows.WindowFor(HitResult.Meh));
addClickActionAssert(0, ClickAction.Hit);
}

private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
{
AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
Expand All @@ -523,6 +573,12 @@ private void addJudgementOffsetAssert(OsuHitObject hitObject, double offset)
() => judgementResults.Single(r => r.HitObject == hitObject).TimeOffset, () => Is.EqualTo(offset).Within(50));
}

private void addJudgementOffsetAssert(string name, Func<OsuHitObject?> hitObject, double offset)
{
AddAssert($"{name} @ judged at {offset}",
() => judgementResults.Single(r => r.HitObject == hitObject()).TimeOffset, () => Is.EqualTo(offset).Within(50));
}

private void addClickActionAssert(int inputIndex, ClickAction action)
=> AddAssert($"input #{inputIndex} resulted in {action}", () => testPolicy.ClickActions[inputIndex], () => Is.EqualTo(action));

Expand Down
52 changes: 52 additions & 0 deletions osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,52 @@ public void TestHitSliderHeadBeforeHitCircle()
addJudgementAssert(hitObjects[1], HitResult.IgnoreHit);
}

[Test]
public void TestInputFallsThroughJudgedSliders()
{
const double time_first_slider = 1000;
const double time_second_slider = 1250;
Vector2 positionFirstSlider = new Vector2(100, 50);
Vector2 positionSecondSlider = new Vector2(100, 80);
var midpoint = (positionFirstSlider + positionSecondSlider) / 2;

var hitObjects = new List<OsuHitObject>
{
new TestSlider
{
StartTime = time_first_slider,
Position = positionFirstSlider,
Path = new SliderPath(PathType.Linear, new[]
{
Vector2.Zero,
new Vector2(25, 0),
})
},
new TestSlider
{
StartTime = time_second_slider,
Position = positionSecondSlider,
Path = new SliderPath(PathType.Linear, new[]
{
Vector2.Zero,
new Vector2(25, 0),
})
}
};

performTest(hitObjects, new List<ReplayFrame>
{
new OsuReplayFrame { Time = time_first_slider, Position = midpoint, Actions = { OsuAction.RightButton } },
new OsuReplayFrame { Time = time_first_slider + 25, Position = midpoint, Actions = { OsuAction.LeftButton } },
new OsuReplayFrame { Time = time_first_slider + 50, Position = midpoint },
});

addJudgementAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, HitResult.Great);
addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0);
addJudgementAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, HitResult.Great);
addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, -200);
}

private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
{
AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
Expand All @@ -354,6 +400,12 @@ private void addJudgementOffsetAssert(OsuHitObject hitObject, double offset)
() => Precision.AlmostEquals(judgementResults.Single(r => r.HitObject == hitObject).TimeOffset, offset, 100));
}

private void addJudgementOffsetAssert(string name, Func<OsuHitObject> hitObject, double offset)
{
AddAssert($"{name} @ judged at {offset}",
() => judgementResults.Single(r => r.HitObject == hitObject()).TimeOffset, () => Is.EqualTo(offset).Within(50));
}

private ScoreAccessibleReplayPlayer currentPlayer;
private List<JudgementResult> judgementResults;

Expand Down
23 changes: 23 additions & 0 deletions osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public void ApplyToDrawableHitObject(DrawableHitObject obj)
head.TrackFollowCircle = !NoSliderHeadMovement.Value;
if (FadeHitCircleEarly.Value && !usingHiddenFading)
applyEarlyFading(head);

if (ClassicNoteLock.Value)
blockInputToUnderlyingObjects(head);

break;

case DrawableSliderTail tail:
Expand All @@ -83,10 +87,29 @@ public void ApplyToDrawableHitObject(DrawableHitObject obj)
case DrawableHitCircle circle:
if (FadeHitCircleEarly.Value && !usingHiddenFading)
applyEarlyFading(circle);

if (ClassicNoteLock.Value)
blockInputToUnderlyingObjects(circle);

break;
}
}

/// <summary>
/// On stable, hitcircles that have already been hit block input from reaching objects that may be underneath them.
/// The purpose of this method is to restore that behaviour.
/// In order to avoid introducing yet another confusing config option, this behaviour is roped into the general notion of "note lock".
/// </summary>
private static void blockInputToUnderlyingObjects(DrawableHitCircle circle)
{
var oldHitAction = circle.HitArea.Hit;
circle.HitArea.Hit = () =>
{
oldHitAction?.Invoke();
return true;
};
}

private void applyEarlyFading(DrawableHitCircle circle)
{
circle.ApplyCustomUpdateState += (dho, state) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public bool OnPressed(KeyBindingPressEvent<OsuAction> e)
case OsuAction.RightButton:
if (IsHovered && (Hit?.Invoke() ?? false))
{
HitAction = e.Action;
HitAction ??= e.Action;
return true;
}

Expand Down