Fix argon volume-aware hitsounds not correctly playing immediately after object placement

Closes https://github.com/ppy/osu/issues/29832.

The underlying reason for the incorrect sample playback was an equality
comparer failure.

Samples are contained in several pools which are managed by the
playfield. In particular, the pools are keyed by `ISampleInfo`
instances. This means that for correct operation, `ISampleInfo` has to
implement `IEquatable<ISampleInfo>` and also provide an appropriately
correct `GetHashCode()` implementation. Different audible samples must
not compare equal to each other when represented by `ISampleInfo`.

As it turns out, `VolumeAwareHitSampleInfo` failed on this, due to not
overriding equality members. Therefore, a `new
HitSampleInfo(HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_NORMAL,
volume: 70)` was allowed to compare equal to a
`VolumeAwareHitSampleInfo` wrapping it, *even though they correspond to
completely different sounds and go through entirely different lookup
path sequences*.

Therefore, to fix, provide more proper equality implementations for
`VolumeAwareHitSampleInfo`.

When testing note that this issue *only occurs immediately after
placing an object*. Saving and re-entering editor makes this issue go
away. I haven't looked too long into why, but the general gist of it is
ordering; it appears that a `normal-hitnormal` pool exists at point
of query of a new object placement, but does not seem to exist when
entering editor afresh. That said I'm not sure that ordering aspect of
this bug matters much if at all, since the two `IHitSampleInfo`s should
never be allowed to alias with each other at all wrt equality.
This commit is contained in:
Bartłomiej Dach 2024-09-23 13:27:36 +02:00
parent 41826d0606
commit e8a394f894
No known key found for this signature in database
2 changed files with 21 additions and 1 deletions

View File

@ -1,6 +1,7 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
using System;
using System.Collections.Generic;
using osu.Game.Audio;
@ -48,5 +49,24 @@ private static string getBank(string originalBank, string sampleName, int volume
return originalBank;
}
}
public override bool Equals(HitSampleInfo? other) => other is VolumeAwareHitSampleInfo && base.Equals(other);
/// <remarks>
/// <para>
/// This override attempts to match the <see cref="Equals"/> override above, but in theory it is not strictly necessary.
/// Recall that <see cref="GetHashCode"/> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.object.gethashcode?view=net-8.0#notes-to-inheritors">must meet the following requirements</a>:
/// </para>
/// <para>
/// "If two objects compare as equal, the <see cref="GetHashCode"/> method for each object must return the same value.
/// However, if two objects do not compare as equal, <see cref="GetHashCode"/> methods for the two objects do not have to return different values."
/// </para>
/// <para>
/// Making this override combine the value generated by the base <see cref="GetHashCode"/> implementation with a constant means
/// that <see cref="HitSampleInfo"/> and <see cref="VolumeAwareHitSampleInfo"/> instances which have the same values of their members
/// will not have equal hash codes, which is slightly more efficient when these objects are used as dictionary keys.
/// </para>
/// </remarks>
public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), 1);
}
}

View File

@ -96,7 +96,7 @@ public virtual IEnumerable<string> LookupNames
public virtual HitSampleInfo With(Optional<string> newName = default, Optional<string> newBank = default, Optional<string?> newSuffix = default, Optional<int> newVolume = default)
=> new HitSampleInfo(newName.GetOr(Name), newBank.GetOr(Bank), newSuffix.GetOr(Suffix), newVolume.GetOr(Volume));
public bool Equals(HitSampleInfo? other)
public virtual bool Equals(HitSampleInfo? other)
=> other != null && Name == other.Name && Bank == other.Bank && Suffix == other.Suffix;
public override bool Equals(object? obj)