ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.32k stars 2.28k forks source link

Mania longnote releases trigger hitsounds #2506

Closed LastExceed closed 4 years ago

LastExceed commented 6 years ago

In stable, this only happens in auto-converts, not in dedicated mania beatmaps. The reason for that is probably that osu!standard sliderends have hitsounds which are kept during the conversion while native mania longnote releases dont. To me this feels inconsistent might thus also be worth looking at in stable.

Anyway, in lazer even native LN releases cause hitsounds, which is different to stable. but before changing anything we should discuss whether we want to keep this the way it is in stable right now:

I personally would suggest keeping the release hitsounds, but make them skinnable - separately to normal hitsounds (maybe the same way combo and score digits can be skinned separately if the user wishes to do so).

Release sounds are something I always missed in mania, as they

Making them skinnable as proposed above brings 2 more advantages:

Rekkonnect commented 6 years ago

There are the cases where the LNs' end does not represent a sound, especially when a song has ended and the LN only signifies the remaining sound. Which brings me to another suggestion: giving the mapper the option to use a hit sound for the beginning, the body and the ending of the LN. The body's hitsound could be customizable in various ways (which could be a backlog feature).

revelsix commented 4 years ago

Long note releases ruin keysounded maps in lazer, this is just one example and also ruin any drums that were designed only to trigger on the start of the LN (drums where there are none, drums hitting twice at the same time, etc.). Both players and mappers have gotten used to silent ends.

LastExceed commented 4 years ago

@revelsix

Long note releases ruin keysounded maps

No they don't. Beatmaps would obviously be able to set release sounds just like they can set hit sounds

drums that were designed only to trigger on the start of the LN

same as above (also why would you map a pointy sound like drums with LN in the first place?)

Both players and mappers have gotten used to silent ends

That is a very conservative view. Opposing change just because of habit is short sighted, we should consider that in a few years from now the majority of players won't even know the old client and how it used to work. Also if you really can't make friends with release sounds you can still disable them via skinning

abraker95 commented 4 years ago

If you can skin them out then it should be fine.

revelsix commented 4 years ago

The current lazer implementation isn't a skinning issue. Maps made before lazer were designed with silent releases in mind, and forcing this option on by default ruins many of those maps. Again, see the first example. It would be fine if mappers could toggle this, but that is not the case as of now.

bdach commented 4 years ago

I looked at this briefly as the related issue is in May milestone. Changing mania converter from:

https://github.com/ppy/osu/blob/59bd2b30356ffef3964216c1ec813bd37409fb58/osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmapConverter.cs#L234-L244

to something like

                    pattern.Add(new HoldNote
                    {
                        StartTime = HitObject.StartTime,
                        Duration = endTimeData.Duration,
                        Column = column,
                        Samples = HitObject.Samples,
                        NodeSamples = (HitObject as IHasRepeats)?.NodeSamples ?? new List<IList<HitSampleInfo>>
                        {
                            HitObject.Samples,
                            new List<HitSampleInfo>()
                        }
                    });

works for that one case, but, ugh. It's probably intensely not-future-proof and I honestly don't even know for sure if it has a chance of even being correct in all cases now.

smoogipoo commented 4 years ago

I'm not super against that solution, fwiw.

bdach commented 4 years ago

In that case I'll submit that tomorrow, possibly with some tests. I would have done so today but I just ran out of usable time in the day.

agausmann commented 4 years ago

Wow, I've been playing mania on lazer for so long that I didn't realize this is a bug, but I certainly noticed when they disappeared.

I've gotten used to the hitsounds at the end of long notes, and I agree with OP's point that it helps with long note timing, which actually matters more in mania than osu. In mania, the ends of long notes are timed the same as normal notes and the starts of long notes, where you are penalized for being too early or too late. Whereas in osu, afaict, you are not penalized for being late on the ends of sliders.

Even though the ends of long notes don't always correlate to a musically-meaningful hitsound, the audible feedback is still useful in my experience, and I'd appreciate some kind of option to restore the old behavior, or to otherwise force some kind of hitsound to trigger on release.

bdach commented 4 years ago

I imagine going forward the mania editor could provide an option to specify node samples (which would include the end sample). The issue with blanket-enabling tail hitsounds is that there are legacy maps that sound straight up broken with them on (see linked issues above), so I'm not sure it should be a user-facing setting.

agausmann commented 4 years ago

I've already made the point that as a user, I don't care if the tails make it sound wrong, I prefer having the feedback, including on the legacy maps I've been playing. I guess editing the maps could be a workaround, but it's not really feasible at scale.

peppy commented 4 years ago

Going to reopen this for further consideration.

peppy commented 4 years ago

A resolution may be to add this to the legacy conversion process up to a certain beatmap vesrion.

peppy commented 4 years ago

Scratch that, opening a new issue otherwise this is going to be confusing to follow.