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

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

Merged
merged 2 commits into from
Sep 8, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ public void TestAutopilotReducesHittableRange()
}

[Test]
[Ignore("Currently broken, first attempt at fixing broke even harder. See https://github.com/ppy/osu/issues/24743.")]
public void TestInputDoesNotFallThroughOverlappingSliders()
{
const double time_first_slider = 1000;
Expand Down Expand Up @@ -549,12 +550,58 @@ public void TestInputDoesNotFallThroughOverlappingSliders()
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);
}

[Test]
public void TestOverlappingObjectsDontBlockEachOtherWhenFullyFadedOut()
{
const double time_first_circle = 1000;
const double time_second_circle = 1200;
const double time_third_circle = 1400;
Vector2 positionFirstCircle = new Vector2(100);
Vector2 positionSecondCircle = new Vector2(200);

var hitObjects = new List<OsuHitObject>
{
new HitCircle
{
StartTime = time_first_circle,
Position = positionFirstCircle,
},
new HitCircle
{
StartTime = time_second_circle,
Position = positionSecondCircle,
},
new HitCircle
{
StartTime = time_third_circle,
Position = positionFirstCircle,
},
};

performTest(hitObjects, new List<ReplayFrame>
{
new OsuReplayFrame { Time = time_first_circle, Position = positionFirstCircle, Actions = { OsuAction.LeftButton } },
new OsuReplayFrame { Time = time_first_circle + 50, Position = positionFirstCircle },
new OsuReplayFrame { Time = time_second_circle, Position = positionSecondCircle, Actions = { OsuAction.LeftButton } },
new OsuReplayFrame { Time = time_second_circle + 50, Position = positionSecondCircle },
new OsuReplayFrame { Time = time_third_circle, Position = positionFirstCircle, Actions = { OsuAction.LeftButton } },
new OsuReplayFrame { Time = time_third_circle + 50, Position = positionFirstCircle },
});

addJudgementAssert(hitObjects[0], HitResult.Great);
addJudgementOffsetAssert(hitObjects[0], 0);

addJudgementAssert(hitObjects[1], HitResult.Great);
addJudgementOffsetAssert(hitObjects[1], 0);

addJudgementAssert(hitObjects[2], HitResult.Great);
addJudgementOffsetAssert(hitObjects[2], 0);
}

private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
{
AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
Expand Down
23 changes: 0 additions & 23 deletions osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ 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 @@ -87,29 +83,10 @@ 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
Loading