143 Commits

• Fix several issues with incorrect sample playback (#35685)

  * Add failing test coverage for layered hit samples not playing in mania when beatmap is converted
  
  Adding the `osu.Game.Rulesets.Osu` reference to the mania test project
  is required so that `HitObjectSampleTest` base logic doesn't die on
  
  https://github.com/ppy/osu/blob/f0aeeeea966f06add12cf2bca3dd48dac8573e82/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs#L88-L91
  
  * Fix layered hit sounds not playing on converted beatmaps in mania
  
  Compare
  https://github.com/peppy/osu-stable-reference/blob/f9e58b4864a10f801393199e7652b2192c7342c3/osu!/GameplayElements/HitObjects/HitObject.cs#L476-L477.
  
  In case of converted beatmaps, the last condition there
  (`BeatmapManager.Current.PlayMode != PlayModes.OsuMania`) fails,
  and thus layered hitsounds are allowed to play.
  
  * Add failing test coverage for mania beatmap conversion assigning wrong samples to spinners
  
  * Fix mania beatmap conversion assigning wrong samples to spinners
  
  A spinner is never `IHasRepeats`. It was a dead condition, leading to
  the hitobject generating fallback `NodeSamples`, which in particular
  feature a silent tail which stable doesn't do.
  
  Noticeably, stable also appears to force the head of the generated hold
  note to have no addition sounds:
  
  https://github.com/peppy/osu-stable-reference/blob/f9e58b4864a10f801393199e7652b2192c7342c3/osu!/GameplayElements/HitObjects/Mania/SpinnerMania.cs#L86-L89
  
  * Add failing test coverage for file hit sample not falling back to plain samples if file missing
  
  * Allow `FileHitSampleInfo` to fall back to standard samples if the file is not found (or not allowed to be looked up)
  
  I'm honestly not 100% as to how closely this matches stable because I
  reached the point wherein I'd rather not look at stable code anymore, so
  as long as this passes tests I'm fine to wait for someone else to report
  new breakage.
  
  * Use alternative workaround for lack of osu! ruleset assembly in mania test project
  
  * Fix encode stability test failures

  Bartłomiej Dach · 2025-11-15 16:19:08 +09:00
  bd4ed49c06
• Adjust gameplay sample models to support custom sample sets

  This is a set of model changes which is supposed to facilitate support
  for custom sample sets to the beatmap editor that is on par with stable.
  
  It is the minimal set of changes. Because of this, it can probably be
  considered "ugly" or however else you want to put it - but before you
  say that, I want to try and pre-empt that criticism by explaining where
  the problems lie.
  
  Problem #1: duality in sample models
  ---
  
  There is currently a weird duality of what a `HitObject`'s samples will
  be.
  
  - If an object has just been placed in the editor, and not saved /
    decoded yet, it will use `HitSampleInfo`.
  
  - If an object has already been encoded to the beatmap at least once, it
    will use `ConvertHitObjectParser.LegacyHitSampleInfo`.
  
  As long as that state of affairs remains, `HitSampleInfo` must be able
  to represent anything that `LegacyHitSampleInfo` can, if feature parity
  is to be achieved.
  
  Problem 2: The 0 & 1 sample banks
  ---
  
  Custom sample banks of 2 and above are a pretty clean affair. They map to
  a suffix on the sample filename, and said samples are allowed to be
  looked up from the beatmap skin. `Suffix` already exists in
  `HitSampleInfo`.
  
  However, the 1 custom sample bank is evil. It uses *non-suffixed*
  samples, *allows lookups from the beatmap skins*, contrary to no bank /
  bank 0, which *also* uses non-suffixed samples, but *doesn't* allow them
  to be looked up from the beatmap skin.
  
  This is why `HitSampleInfo.UseBeatmapSamples` has been called to
  existence - without it there is no way to represent the ability of using
  or not using the beatmap skin assets.
  
  As has been stated previously in discussions about this feature, it's
  both a *mapping* and a *skinning* concern.
  
  There are many things you could do about either of these problems, but I
  am pretty sure tackling either one is going to take *many* more lines of
  code than this commit does. Which is why this is the starting point of
  negotiation.

  Bartłomiej Dach · 2025-10-22 14:58:08 +02:00
  c361e6d3c2
• Disallow zero-length sliders from specifying a non-zero number of repeats (#35220)
  Bartłomiej Dach · 2025-10-08 16:01:04 +09:00
  1a522a19f1
• Fix lack of slider encode-decode stability due to truncating control point coordinates

  Mostly closes https://github.com/ppy/osu/issues/33505.
  
  Compare
  
  	https://github.com/ppy/osu/blob/97e6187f0d7c3dbee896596a623e34627135bf0e/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs#L56-L59
  
  I say "mostly" here because I'm rather skeptical that this is 100% rock
  solid still, for one reason - namely that the game stores path control
  point coordinates relative to the head, then turns them into absolute
  coordinates when encoding, and then on decoding turns them back into
  coordinates relative to the head, which in floating-point world is a Bad
  Idea because of round-off error. But I'm not fixing that without
  introducing a completely new beatmap format or rewriting half the
  editor to address that, so I'll just pretend that I don't know any
  of this until someone notices.

  Bartłomiej Dach · 2025-06-10 14:09:13 +02:00
  eab02a2aa5
• Apply minor performance rules
  Huo Yaoyuan · 2024-11-28 20:41:44 +08:00
  f5a7716509
• Remove unused params
  Dan Balasescu · 2024-11-11 16:10:15 +09:00
  8c68db0a36
• Add IHasLegacyHitObjectType to ConvertHitObject
  Dan Balasescu · 2024-11-11 16:10:15 +09:00
  7206e97b7b
• Merge implementations of ConvertHitObjectParser

  Having these be separate implementations sounded awesome at the time,
  but it only ever led to confusion. There's no practical difference if,
  for example, catch sees hitobjects with `IHasPosition` instead of
  `IHasXPosition`.

  Dan Balasescu · 2024-11-11 15:09:13 +09:00
  e1d93a7d9c
• Fix hitobjects without custom sample banks parsing as not auto sample bank
  OliBomby · 2024-10-01 12:11:44 +02:00
  2381c2c72c
• Implement auto additions editor-only
  OliBomby · 2024-10-01 16:40:48 +09:00
  0a78eb9628
• add back protection against perfect curve segments with > 3 points
  OliBomby · 2024-08-21 12:03:15 +02:00
  eefd7cf083
• Fix perfect curve anchors losing type between reloads
  OliBomby · 2024-08-16 14:24:55 +02:00
  a2e26ba9ff
• Fix hit object coordinates being truncated to int values

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

  Dean Herbert · 2024-08-09 16:34:39 +09:00
  d072c6a743
• Update HasFlag usages
  Dean Herbert · 2024-07-03 00:19:04 +09:00
  5350945340
• Replace usages of is null with == null
  Dean Herbert · 2024-04-03 01:50:39 +08:00
  94cbe1838f
• Use Slice method instead of index range operators for readability
  Salman Ahmed · 2024-03-19 02:43:36 +03:00
  a8ce6a0bba
• Make the code more clear
  Huo Yaoyuan · 2024-02-29 20:02:04 +08:00
  a11e63b184
• Fix method indent size

  Co-authored-by: Salman Ahmed <frenzibyte@gmail.com>

  Huo Yaoyuan · 2024-02-29 10:47:16 +08:00
  f28f19ed7e
• Fix formatting
  Huo Yaoyuan · 2024-02-29 00:24:24 +08:00
  e86ebd6cdb
• The overhead of LINQ is not ignorable
  Huo Yaoyuan · 2024-02-29 00:07:00 +08:00
  470d2be2e1
• Use ArrayPool instead of stackalloc
  Huo Yaoyuan · 2024-02-28 22:51:36 +08:00
  bcb91f348d
• Update parsing.
  Huo Yaoyuan · 2024-02-28 22:42:08 +08:00
  fe34577ee2
• Parse points and segments for path string
  Huo Yaoyuan · 2024-02-28 22:01:39 +08:00
  f49aa4d815
• Fix incorrect parsing of BSpline curve types
  Dean Herbert · 2023-11-21 15:38:12 +09:00
  3d094f84ad
• Use static BEZIER instead of allocating new every time
  Bartłomiej Dach · 2023-11-20 13:35:07 +09:00
  80a3225bb2
• Fix code style/quality issues
  cs · 2023-11-13 08:25:27 +01:00
  fa976a5aa0
• CI Fixup
  cs · 2023-11-11 15:02:06 +01:00
  54b8244a18
• Generalize Bezier curves to BSplines of Nth degree
  Thomas Müller-Höhne · 2023-11-11 13:32:53 +01:00
  926636cc03
• Replace sample bank fix with more correct fix

  stable does not treat unknown enum members as `None` / `Auto`, it treats
  them as `Normal`:
  
      switch (sampleSet)
      {
          case SampleSet.Normal:
          default:
              sample = 0;
              break;
          case SampleSet.None:
          case SampleSet.Soft:
              sample = 1;
              break;
          case SampleSet.Drum:
              sample = 2;
              break;
      }
  
      (from https://github.com/peppy/osu-stable-reference/blob/1531237b63392e82c003c712faa028406073aa8f/osu!/Audio/AudioEngine.cs#L1158-L1171).

  Bartłomiej Dach · 2023-09-19 13:53:49 +02:00
  c4a0ca326e
• Default to none bank if invalid samplebank is specified
  sw1tchbl4d3 · 2023-09-13 12:44:00 +02:00
  5b2af7f264
• Manual fixes to reduce warnings to zero
  Dean Herbert · 2023-06-24 01:52:53 +09:00
  df5b389629
• Remove redundant nullable suppression directives
  Salman Ahmed · 2023-06-07 08:20:41 +03:00
  62cb6a98ca
• Fix weird optional usage in HitSampleInfo.With
  Dean Herbert · 2023-05-17 14:11:52 +09:00
  510ebe1f23
• Make HitSampleInfo.Bank non-nullable
  Dean Herbert · 2023-05-16 18:12:56 +09:00
  83dcd78826
• fix legacy parser incorrectly assigning sample info for sliders
  OliBomby · 2023-04-29 23:51:49 +02:00
  ffcc8e91b2
• Always add to additions soundbank in convertSoundType
  sw1tchbl4d3 · 2023-02-09 22:27:52 +01:00
  f0d11f44fc
• Rename some pieces and better document SampleBankInfo
  Dean Herbert · 2022-10-25 14:55:33 +09:00
  da93849b80
• Manual #nullable processing
  Dan Balasescu · 2022-06-17 16:38:35 +09:00
  fd9902e816
• Fix Catmull slider parsing with duplicate initial point
  Dan Balasescu · 2022-05-24 11:47:42 +09:00
  9259aa94f1
• Don't merge adjacent legacy Catmull segments
  Dan Balasescu · 2022-05-18 17:16:24 +09:00
  731f0960ec
• Add comment regarding equality check importance in LegacyHitSampleInfo
  Dean Herbert · 2022-03-23 20:18:44 +09:00
  c079a9cd32
• Revert "Remove IsLayered from LegacyHitSampleInfo comparison"

  This reverts commit 4523393208.

  Dean Herbert · 2022-03-23 20:15:17 +09:00
  997c091a8d
• Revert "Remove IsLayered from GetHasCode implementation"

  This reverts commit 16ee6b5fc7.

  Dean Herbert · 2022-03-23 20:15:13 +09:00
  2ea9e5245c
• Remove IsLayered from GetHasCode implementation
  Dean Herbert · 2022-03-14 18:12:04 +09:00
  16ee6b5fc7
• Remove IsLayered from LegacyHitSampleInfo comparison

  The equality of samples is generally used to compare the sample
  equality, not its full properties. For instance, we don't compare
  `Volume` in the base implementation.
  
  Having `IsLayered` here breaks actual usages of equality, ie. for
  pooling purposes.

  Dean Herbert · 2022-03-14 17:19:48 +09:00
  4523393208
• Fix most open compound words in identifiers being closed
  Joseph Madamba · 2021-12-27 20:26:28 -08:00
  7de43e3aba
• Merge pull request #15255 from goodtrailer/node-samples-ilist

  Change IHasRepeats.NodeSamples to IList from List

  Dean Herbert · 2021-10-28 14:24:26 +09:00
  5c7623e68e
• Merge branch 'master' into fix-skin-layout-editor-crash
  Dean Herbert · 2021-10-28 06:24:24 +09:00
  add39fbd4f
• Apply batch fixing of built-in types using var
  Dean Herbert · 2021-10-27 13:04:41 +09:00
  6944151486
• Change IHasRepeats.NodeSamples to IList from List
  goodtrailer · 2021-10-23 01:59:07 -07:00
  4440b9ca11