Skip to content

Commit

Permalink
Fix GameplaySampleTriggerSource not considering nested objects when…
Browse files Browse the repository at this point in the history
… determining the best sample to play
  • Loading branch information
peppy committed Feb 16, 2023
1 parent 05b2c56 commit fe6c13a
Showing 1 changed file with 51 additions and 16 deletions.
67 changes: 51 additions & 16 deletions osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
Expand Up @@ -7,6 +7,7 @@
using osu.Framework.Graphics.Containers;
using osu.Game.Audio;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Skinning;

namespace osu.Game.Rulesets.UI
Expand Down Expand Up @@ -68,27 +69,61 @@ public virtual void Play()
protected HitObject GetMostValidObject()
{
// The most optimal lookup case we have is when an object is alive. There are usually very few alive objects so there's no drawbacks in attempting this lookup each time.
var hitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true)?.HitObject;
var drawableHitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true);

if (drawableHitObject != null)
{
// A hit object may have a more valid nested object.
drawableHitObject = getMostValidNestedDrawable(drawableHitObject);

return drawableHitObject.HitObject;
}

// In the case a next object isn't available in drawable form, we need to do a somewhat expensive traversal to get a valid sound to play.
if (hitObject == null)
// This lookup can be skipped if the last entry is still valid (in the future and not yet hit).
if (fallbackObject == null || fallbackObject.Result?.HasResult == true)
{
// This lookup can be skipped if the last entry is still valid (in the future and not yet hit).
if (fallbackObject == null || fallbackObject.Result?.HasResult == true)
{
// We need to use lifetime entries to find the next object (we can't just use `hitObjectContainer.Objects` due to pooling - it may even be empty).
// If required, we can make this lookup more efficient by adding support to get next-future-entry in LifetimeEntryManager.
fallbackObject = hitObjectContainer.Entries
.Where(e => e.Result?.HasResult != true).MinBy(e => e.HitObject.StartTime);

// In the case there are no unjudged objects, the last hit object should be used instead.
fallbackObject ??= hitObjectContainer.Entries.LastOrDefault();
}

hitObject = fallbackObject?.HitObject;
// We need to use lifetime entries to find the next object (we can't just use `hitObjectContainer.Objects` due to pooling - it may even be empty).
// If required, we can make this lookup more efficient by adding support to get next-future-entry in LifetimeEntryManager.
fallbackObject = hitObjectContainer.Entries
.Where(e => e.Result?.HasResult != true).MinBy(e => e.HitObject.StartTime);

if (fallbackObject != null)
return fallbackObject.HitObject;

// In the case there are no unjudged objects, the last hit object should be used instead.
fallbackObject ??= hitObjectContainer.Entries.LastOrDefault();
}

return hitObject;
if (fallbackObject == null)
return null;

bool fallbackHasResult = fallbackObject.Result?.HasResult == true;

// If the fallback has been judged then we want the sample from the object itself.
if (fallbackHasResult)
return fallbackObject.HitObject;

// Else we want the earliest (including nested).
// In cases of nested objects, they will always have earlier sample data than their parent object.
return getEarliestNestedObject(fallbackObject.HitObject);
}

private DrawableHitObject getMostValidNestedDrawable(DrawableHitObject o)
{
var nestedWithoutResult = o.NestedHitObjects.FirstOrDefault(n => n.Result?.HasResult != true);

if (nestedWithoutResult == null)
return o;

return getMostValidNestedDrawable(nestedWithoutResult);
}

private HitObject getEarliestNestedObject(HitObject hitObject)
{
var nested = hitObject.NestedHitObjects.FirstOrDefault();

return nested != null ? getEarliestNestedObject(nested) : hitObject;
}

private SkinnableSound getNextSample()
Expand Down

0 comments on commit fe6c13a

Please sign in to comment.