Skip to content

Add slider head circle late hit lenience #25776

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Dec 18, 2023
Merged

Add slider head circle late hit lenience #25776

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
599fdb0
Add lenience for late-hit of slider heads
smoogipoo Dec 14, 2023
6bd190c
Refactor all slider input into SliderInputManager
smoogipoo Dec 15, 2023
3b8a73b
Refactor test
smoogipoo Dec 15, 2023
1221001
Use the cursor position to test nested object validity
smoogipoo Dec 15, 2023
9ae3be8
Add some text to the test scene showing hits/misses
smoogipoo Dec 15, 2023
acb7016
Remove unused using
smoogipoo Dec 15, 2023
c0e9692
Merge branch 'master' into slider-late-hit-lenience
peppy Dec 17, 2023
f77884b
Only hit passed-through ticks if none were missed
smoogipoo Dec 17, 2023
fbe48d7
Fix tail being missed too early
smoogipoo Dec 17, 2023
9b02bd7
Only track if in slider ball after any ticks missed
smoogipoo Dec 17, 2023
fddfa33
Fix 1-frame issues due to referencing external value
smoogipoo Dec 17, 2023
2b33aec
Require slider head to be judged before ticks
smoogipoo Dec 17, 2023
04d5421
Fix assertions
smoogipoo Dec 17, 2023
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
460 changes: 460 additions & 0 deletions osu.Game.Rulesets.Osu.Tests/TestSceneSliderLateHitJudgement.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
{
public partial class DrawableHitCircle : DrawableOsuHitObject, IHasApproachCircle
{
public OsuAction? HitAction => HitArea.HitAction;
public OsuAction? HitAction => HitArea?.HitAction;
protected virtual OsuSkinComponents CirclePieceComponent => OsuSkinComponents.HitCircle;

public SkinnableDrawable ApproachCircle { get; private set; }
Expand Down
5 changes: 5 additions & 0 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ protected override void UpdateInitialTransforms()
/// </summary>
public virtual void Shake() { }

/// <summary>
/// Causes this <see cref="DrawableOsuHitObject"/> to get hit, disregarding all conditions in implementations of <see cref="DrawableHitObject.CheckForResult"/>.
/// </summary>
public void HitForcefully() => ApplyResult(r => r.Type = r.Judgement.MaxResult);

/// <summary>
/// Causes this <see cref="DrawableOsuHitObject"/> to get missed, disregarding all conditions in implementations of <see cref="DrawableHitObject.CheckForResult"/>.
/// </summary>
Expand Down
12 changes: 8 additions & 4 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public partial class DrawableSlider : DrawableOsuHitObject
public IBindable<int> PathVersion => pathVersion;
private readonly Bindable<int> pathVersion = new Bindable<int>();

public readonly SliderInputManager SliderInputManager;

private Container<DrawableSliderHead> headContainer;
private Container<DrawableSliderTail> tailContainer;
private Container<DrawableSliderTick> tickContainer;
Expand All @@ -72,9 +74,10 @@ public DrawableSlider()
public DrawableSlider([CanBeNull] Slider s = null)
: base(s)
{
SliderInputManager = new SliderInputManager(this);

Ball = new DrawableSliderBall
{
GetInitialHitAction = () => HeadCircle.HitAction,
BypassAutoSizeAxes = Axes.Both,
AlwaysPresent = true,
Alpha = 0
Expand All @@ -88,6 +91,7 @@ private void load()

AddRangeInternal(new Drawable[]
{
SliderInputManager,
shakeContainer = new ShakeContainer
{
ShakeDuration = 30,
Expand Down Expand Up @@ -232,7 +236,7 @@ protected override void Update()
{
base.Update();

Tracking.Value = Ball.Tracking;
Tracking.Value = SliderInputManager.Tracking;

if (Tracking.Value && slidingSample != null)
// keep the sliding sample playing at the current tracking position
Expand All @@ -245,8 +249,8 @@ protected override void Update()

foreach (DrawableHitObject hitObject in NestedHitObjects)
{
if (hitObject is ITrackSnaking s) s.UpdateSnakingPosition(HitObject.Path.PositionAt(SliderBody?.SnakedStart ?? 0), HitObject.Path.PositionAt(SliderBody?.SnakedEnd ?? 0));
if (hitObject is IRequireTracking t) t.Tracking = Ball.Tracking;
if (hitObject is ITrackSnaking s)
s.UpdateSnakingPosition(HitObject.Path.PositionAt(SliderBody?.SnakedStart ?? 0), HitObject.Path.PositionAt(SliderBody?.SnakedEnd ?? 0));
}

Size = SliderBody?.Size ?? Vector2.Zero;
Expand Down
119 changes: 1 addition & 118 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,9 @@
#nullable disable

using System;
using System.Collections.Generic;
using System.Linq;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input;
using osu.Framework.Input.Events;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Osu.Skinning.Default;
Expand All @@ -21,13 +16,10 @@

namespace osu.Game.Rulesets.Osu.Objects.Drawables
{
public partial class DrawableSliderBall : CircularContainer, ISliderProgress, IRequireHighFrequencyMousePosition
public partial class DrawableSliderBall : CircularContainer, ISliderProgress
{
public const float FOLLOW_AREA = 2.4f;

public Func<OsuAction?> GetInitialHitAction;

private Drawable followCircleReceptor;
private DrawableSlider drawableSlider;
private Drawable ball;

Expand All @@ -48,13 +40,6 @@ private void load(DrawableHitObject drawableSlider)
Anchor = Anchor.Centre,
RelativeSizeAxes = Axes.Both,
},
followCircleReceptor = new CircularContainer
{
Origin = Anchor.Centre,
Anchor = Anchor.Centre,
RelativeSizeAxes = Axes.Both,
Masking = true
},
ball = new SkinnableDrawable(new OsuSkinComponentLookup(OsuSkinComponents.SliderBall), _ => new DefaultSliderBall())
{
Anchor = Anchor.Centre,
Expand All @@ -63,14 +48,6 @@ private void load(DrawableHitObject drawableSlider)
};
}

private Vector2? lastScreenSpaceMousePosition;

protected override bool OnMouseMove(MouseMoveEvent e)
{
lastScreenSpaceMousePosition = e.ScreenSpaceMousePosition;
return base.OnMouseMove(e);
}

public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null)
{
// Consider the case of rewinding - children's transforms are handled internally, so propagating down
Expand All @@ -86,100 +63,6 @@ public override void ApplyTransformsAt(double time, bool propagateChildren = fal
base.ApplyTransformsAt(time, false);
}

private bool tracking;

public bool Tracking
{
get => tracking;
private set
{
if (value == tracking)
return;

tracking = value;

followCircleReceptor.Scale = new Vector2(tracking ? FOLLOW_AREA : 1f);
}
}

/// <summary>
/// If the cursor moves out of the ball's radius we still need to be able to receive positional updates to stop tracking.
/// </summary>
public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true;

/// <summary>
/// The point in time after which we can accept any key for tracking. Before this time, we may need to restrict tracking to the key used to hit the head circle.
///
/// This is a requirement to stop the case where a player holds down one key (from before the slider) and taps the second key while maintaining full scoring (tracking) of sliders.
/// Visually, this special case can be seen below (time increasing from left to right):
///
/// Z Z+X Z
/// o========o
///
/// Without this logic, tracking would continue through the entire slider even though no key hold action is directly attributing to it.
///
/// In all other cases, no special handling is required (either key being pressed is allowable as valid tracking).
///
/// The reason for storing this as a time value (rather than a bool) is to correctly handle rewind scenarios.
/// </summary>
private double? timeToAcceptAnyKeyAfter;

/// <summary>
/// The actions that were pressed in the previous frame.
/// </summary>
private readonly List<OsuAction> lastPressedActions = new List<OsuAction>();

protected override void Update()
{
base.Update();

// from the point at which the head circle is hit, this will be non-null.
// it may be null if the head circle was missed.
var headCircleHitAction = GetInitialHitAction();

if (headCircleHitAction == null)
timeToAcceptAnyKeyAfter = null;

var actions = drawableSlider.OsuActionInputManager?.PressedActions;

// if the head circle was hit with a specific key, tracking should only occur while that key is pressed.
if (headCircleHitAction != null && timeToAcceptAnyKeyAfter == null)
{
var otherKey = headCircleHitAction == OsuAction.RightButton ? OsuAction.LeftButton : OsuAction.RightButton;

// we can start accepting any key once all other keys have been released in the previous frame.
if (!lastPressedActions.Contains(otherKey))
timeToAcceptAnyKeyAfter = Time.Current;
}

Tracking =
// even in an edge case where current time has exceeded the slider's time, we may not have finished judging.
// we don't want to potentially update from Tracking=true to Tracking=false at this point.
(!drawableSlider.AllJudged || Time.Current <= drawableSlider.HitObject.GetEndTime())
// in valid position range
&& lastScreenSpaceMousePosition.HasValue && followCircleReceptor.ReceivePositionalInputAt(lastScreenSpaceMousePosition.Value) &&
// valid action
(actions?.Any(isValidTrackingAction) ?? false);

lastPressedActions.Clear();
if (actions != null)
lastPressedActions.AddRange(actions);
}

/// <summary>
/// Check whether a given user input is a valid tracking action.
/// </summary>
private bool isValidTrackingAction(OsuAction action)
{
bool headCircleHit = GetInitialHitAction().HasValue;

// if the head circle was hit, we may not yet be allowed to accept any key, so we must use the initial hit action.
if (headCircleHit && (!timeToAcceptAnyKeyAfter.HasValue || Time.Current <= timeToAcceptAnyKeyAfter.Value))
return action == GetInitialHitAction();

return action == OsuAction.LeftButton || action == OsuAction.RightButton;
}

private Vector2? lastPosition;

public void UpdateProgress(double completionProgress)
Expand Down
6 changes: 6 additions & 0 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderHead.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ protected override void OnApply()
CheckHittable = (d, t, r) => DrawableSlider.CheckHittable?.Invoke(d, t, r) ?? ClickAction.Hit;
}

protected override void CheckForResult(bool userTriggered, double timeOffset)
{
base.CheckForResult(userTriggered, timeOffset);
DrawableSlider.SliderInputManager.PostProcessHeadJudgement(this);
}

protected override HitResult ResultFor(double timeOffset)
{
Debug.Assert(HitObject != null);
Expand Down
20 changes: 2 additions & 18 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

namespace osu.Game.Rulesets.Osu.Objects.Drawables
{
public partial class DrawableSliderRepeat : DrawableOsuHitObject, ITrackSnaking, IRequireTracking
public partial class DrawableSliderRepeat : DrawableOsuHitObject, ITrackSnaking
{
public new SliderRepeat HitObject => (SliderRepeat)base.HitObject;

Expand All @@ -36,8 +36,6 @@ public partial class DrawableSliderRepeat : DrawableOsuHitObject, ITrackSnaking,

public override bool DisplayResult => false;

public bool Tracking { get; set; }

public DrawableSliderRepeat()
: base(null)
{
Expand Down Expand Up @@ -85,21 +83,7 @@ protected override void OnApply()
Position = HitObject.Position - DrawableSlider.Position;
}

protected override void CheckForResult(bool userTriggered, double timeOffset)
{
// shared implementation with DrawableSliderTick.
if (timeOffset >= 0)
{
// Attempt to preserve correct ordering of judgements as best we can by forcing
// an un-judged head to be missed when the user has clearly skipped it.
//
// This check is applied to all nested slider objects apart from the head (ticks, repeats, tail).
if (Tracking && !DrawableSlider.HeadCircle.Judged)
DrawableSlider.HeadCircle.MissForcefully();

ApplyResult(r => r.Type = Tracking ? r.Judgement.MaxResult : r.Judgement.MinResult);
}
}
protected override void CheckForResult(bool userTriggered, double timeOffset) => DrawableSlider.SliderInputManager.TryJudgeNestedObject(this, timeOffset);

protected override void UpdateInitialTransforms()
{
Expand Down
37 changes: 2 additions & 35 deletions osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,18 @@
#nullable disable

using System.Diagnostics;
using System.Linq;
using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Skinning;
using osuTK;

namespace osu.Game.Rulesets.Osu.Objects.Drawables
{
public partial class DrawableSliderTail : DrawableOsuHitObject, IRequireTracking
public partial class DrawableSliderTail : DrawableOsuHitObject
{
public new SliderTailCircle HitObject => (SliderTailCircle)base.HitObject;

Expand All @@ -37,8 +35,6 @@ public partial class DrawableSliderTail : DrawableOsuHitObject, IRequireTracking
/// </summary>
public bool SamplePlaysOnlyOnHit { get; set; } = true;

public bool Tracking { get; set; }

public SkinnableDrawable CirclePiece { get; private set; }

private Container scaleContainer;
Expand Down Expand Up @@ -125,36 +121,7 @@ protected override void UpdateHitStateTransforms(ArmedState state)
}
}

protected override void CheckForResult(bool userTriggered, double timeOffset)
{
if (userTriggered)
Copy link
Contributor

@Zyfarok Zyfarok Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure what this test was doing* but better be safe than sorry:
Is this test no longer needed ?

(*: I looked at the doc of UpdateResult but still didn't get what userTriggered means exactly)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

userTriggered = true comes from user input, if somewhere UpdateResult(true) was called.

It's irrelevant here because that's not called.

return;

// Ensure the tail can only activate after all previous ticks/repeats already have.
//
// This covers the edge case where the lenience may allow the tail to activate before
// the last tick, changing ordering of score/combo awarding.
var lastTick = DrawableSlider.NestedHitObjects.LastOrDefault(o => o.HitObject is SliderTick || o.HitObject is SliderRepeat);
if (lastTick?.Judged == false)
return;

if (timeOffset < SliderEventGenerator.TAIL_LENIENCY)
return;

// Attempt to preserve correct ordering of judgements as best we can by forcing
// an un-judged head to be missed when the user has clearly skipped it.
//
// This check is applied to all nested slider objects apart from the head (ticks, repeats, tail).
if (Tracking && !DrawableSlider.HeadCircle.Judged)
DrawableSlider.HeadCircle.MissForcefully();

// The player needs to have engaged in tracking at any point after the tail leniency cutoff.
// An actual tick miss should only occur if reaching the tick itself.
if (Tracking)
ApplyResult(r => r.Type = r.Judgement.MaxResult);
else if (timeOffset > 0)
ApplyResult(r => r.Type = r.Judgement.MinResult);
}
protected override void CheckForResult(bool userTriggered, double timeOffset) => DrawableSlider.SliderInputManager.TryJudgeNestedObject(this, timeOffset);

protected override void OnApply()
{
Expand Down
Loading