ppy / osu

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

Result of applying Hidden and Freeze Frame mods in osu! ruleset depends on application order #21471

Open bdach opened 1 year ago

bdach commented 1 year ago

Type

Game behaviour

Bug description

This was reported to me on the performance points discord.

Reproduction steps via "pp dev" pathway:

  1. Checkout https://github.com/apollo-dw/osu/tree/cognition (@apollo-dw's WIP branch)
  2. Download https://osu.ppy.sh/beatmapsets/725171#osu/1541573
  3. From osu-tools root directory, run UseLocalOsu.{ps1,sh} to use branch from step (1)
  4. Run the difficulty subcommand with Freeze Frame and Hidden and the difficulty linked in step (2). Call it as -nc -m fr -m hd once and -nc -m hd -m fr (with order inverted) the other time. You should see:

1669829852


The reason that this is happening is a data dependency. Freeze Frame modifies TimePreempt of hitobjects:

https://github.com/ppy/osu/blob/54c0b2c20c3209f1f17be81b8883ef66ec8a83bb/osu.Game.Rulesets.Osu/Mods/OsuModFreezeFrame.cs#L53

while Hidden uses TimePreempt to set TimeFadeIn:

https://github.com/ppy/osu/blob/54c0b2c20c3209f1f17be81b8883ef66ec8a83bb/osu.Game.Rulesets.Osu/Mods/OsuModHidden.cs#L45

Depending on which mod is applied first, TimeFadeIn may be set to a different value.

I don't know what the answer is here. The knee-jerk response is "the two mods are incompatible". The hack answer is "the application order should be chosen arbitrarily and enforced for all mods". The overengineered answer is "something should be detecting data dependencies such as this and deciding the correct order of application via a DAG of the data dependency graph".

Semi-related: https://github.com/ppy/osu/issues/16258

Screenshots or videos

No response

Version

current master

Logs

n/a

mk56-spn commented 1 year ago

linking https://github.com/ppy/osu/pull/20044 and https://github.com/ppy/osu/pull/18559 since id wager the same is going to happen to them with random mod if they get merged ( but in this case the issue should occur with hitobject placement)

I kinda foresaw this headache but I didn't think there was an existing case already

edit : https://github.com/ppy/osu/discussions/21578#discussioncomment-4342762 another potential case