- Closes https://github.com/ppy/osu/issues/37757
Commit-by-commit reading is recommended. Commits will be split to PRs on
request but I consider this to be the minimal viable functional
increment.
## Done
- This adds a first version of a full storyboard encoder
(a66dc406f498e35d4e0c8f2a462e946a9a1aeccc). I expect there to be hiccups
due to weird corners of the `.osb` format; this is only intended to be
somewhat correct as a start to build upon. Storyboarders are asked to
file issues as necessary.
- Due to the fact that storyboard definitions can reside both in the
`.osu` and the `.osb`, b60698a95c4de1bfeb36fbb159fd5a6028920832 adds the
required storage to be able to tell which storyboard element lives
where, so that it can be decoded properly later.
- In c9d3e04a4135886b5b0943c85f3cc6f4fe99c84c, the storyboard decoder is
weaved into the beatmap decoder to handle the `.osu` part of the
storyboard, via the
`LegacyStoryboardEncoder.Encode{General,Events}ToBeatmap()` methods. For
`.osb`s, `LegacyStoryboardEncoder.EncodeStandaloneStoryboard()` is
intended, but for now is not used outside tests.
- Because of the above, dd1c4e43dc51154cd67860f096712f8b4f229661 removes
`Beatmap.UnhandledEventLines` as no longer required.
- 26ac417ed98a8937c42e5f52c4e15ef065a48902 adds tests. They are mostly
handwritten to ensure basic encode-decode roundtripping. Using existing
storyboards is difficult, see "Known issues" section as to why.
- 5cc542366db7caac38eb0729260d884905a2c0d5 fixes a bug in the storyboard
decoder where the trigger group number was not properly negated on
decode (see inline comment reference to relevant stable code).
## Known issues
- Any and all variables in the `[Variables]` section are inlined into
their usages by `LegacyStoryboardDecoder`, and as such
`LegacyStoryboardEncoder` will end up inlining them and discarding the
`[Variables]` section. As far as I can tell stable will also do this.
- `LegacyStoryboardDecoder` splits all `M` (move) commands into
`MX`/`MY` commands. Therefore, `LegacyStoryboardEncoder` will write out
things in the same split way. I did not put in effort to attempt to
reconcile this, for reasons of part laziness, part not wanting to bloat
this already-large diff.
- Ordering of storyboard samples on decode may not match the order on
decode. I'm crossing fingers this doesn't matter.
Closes https://github.com/ppy/osu/issues/37232.
The actual fix is
https://github.com/ppy/osu/commit/e959b20517497a093d3c00a17457c5d36bf57651;
everything else is window dressing / test harness to ensure I don't try
and do a wrong change like https://github.com/ppy/osu/pull/37251 did. I
recommend reviewing commit-by-commit.
See [this desmos](https://www.desmos.com/calculator/a5yjpacvxa) for
visual explanation of change, I think it does a better job at explaining
this than any words I could type here.
Of note:
- In the end this did only affect 14K but that should never be assumed
when floating point is involved.
- Test cases generated here were generated in stable manually.
- Except for 11 / 13 / 15 / 17K which are not officially supported and
which don't work in lazer due to orthogonal reasons (see comment added
in this PR in `ManiaBeatmapConverter`), decoding in lazer was always
fine.
- My worry was that the old encoding method before this PR could
potentially cause stable to move a note from one column to another but
thankfully that is not the case. The old method of encoding columns as X
positions does not cause issues wherein lazer reads them back
differently than stable after encode.
I checked this by checking out `master`, re-encoding all of the test
stair-pattern nK beatmaps added in this PR on `master`, exporting that
as compatibility, re-importing to stable, and cross-checking that the
decoded beatmap is visually the same on lazer and on stable.
This is important to check because if this wasn't the case, we'd
potentially have cases of actual online beatmaps (remember that we have
BSS now) wherein a beatmap plays differently on stable than on lazer due
to notes moving between columns, and would need to screen for this being
the case and potentially apply corrective / reconciliatory action.
Bartłomiej Dach ·