Merge pull request #29970 from bdach/volume-aware-sample-equality-pitfall

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

osu.Game.Rulesets.Taiko.Tests/VolumeAwareHitSampleInfoTest.cs

@@ -0,0 +1,27 @@
+// 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 NUnit.Framework;
+using osu.Game.Audio;
+using osu.Game.Rulesets.Taiko.Skinning.Argon;
+
+namespace osu.Game.Rulesets.Taiko.Tests
+{
+    [TestFixture]
+    public class VolumeAwareHitSampleInfoTest
+    {
+        [Test]
+        public void TestVolumeAwareHitSampleInfoIsNotEqualToItsUnderlyingSample(
+            [Values(HitSampleInfo.HIT_NORMAL, HitSampleInfo.HIT_CLAP)]
+            string sample,
+            [Values(HitSampleInfo.BANK_NORMAL, HitSampleInfo.BANK_SOFT)]
+            string bank,
+            [Values(30, 70, 100)] int volume)
+        {
+            var underlyingSample = new HitSampleInfo(sample, bank, volume: volume);
+            var volumeAwareSample = new VolumeAwareHitSampleInfo(underlyingSample);
+
+            Assert.That(underlyingSample, Is.Not.EqualTo(volumeAwareSample));
+        }
+    }
+}

osu.Game.Rulesets.Taiko/Skinning/Argon/VolumeAwareHitSampleInfo.cs

@@ -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 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Argon
                     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);
     }
 }

osu.Game/Audio/HitSampleInfo.cs

@@ -96,7 +96,7 @@ namespace osu.Game.Audio
         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)