Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beatmap/BeatmapPlayback refactor #515

Merged
merged 7 commits into from
Jan 8, 2022

Conversation

123jimin
Copy link

@123jimin 123jimin commented Oct 6, 2021

Originally planned to split #445 into two PRs, but realized that adding supports for scroll speeds is quite small yet necessary for the refactoring.

  • Stops are replaced by scroll speed changes.
  • Support for scroll speed changes are added, but (other than x1 and x0) it's not stable yet.
  • Camera zooms, scroll speed changes, and center splits are now managed by LineGraph, which is future-compatible with KSON.
  • Reduced passing raw pointers and whole vectors (m_objectStates)
  • Rewrote the part of BeatmapPlayback::Update where passed objects are checked.
  • Rewrote the part initializing TimingPoints during reading ksh file
  • Internalized various beatmap-related functions, such as computing the mode BPM and getting the first/last hittable object.

There's one side-effect to this PR:

  • The animation for the background of gameplay stops whenever there's a stop.
    • The reason behind this change is that the timescale of the background animation is now multiplied by the scroll speed, which is zero whenever there's a stop.
    • This behavior is conforming to SDVX, but this change may effect other skins.

@123jimin 123jimin marked this pull request as ready for review October 7, 2021 06:43
@123jimin
Copy link
Author

123jimin commented Dec 1, 2021

In 3ddcfa3, I re-implemented the stops so that they work like this now:

  • If a stop region is not overlapping with another stop region, then the scroll speed inside the region will be set to 0.
  • If a stop region is overlapped with another stop region, then the scroll speed inside the region will be decreased by 1.

This makes using stops and scroll_speeds together behave nicely, and more importantly old charts with gimmicks implemented by overlapping stop regions (which will result in negative scroll speed in the current version of USC) will still behave identically after this PR gets merged.

@123jimin
Copy link
Author

123jimin commented Dec 1, 2021

Also, I added scroll_speed because it's trivial to do so (only 5 lines of code) and it's useful for debugging, but the parameter is non-standard and experimental, so don't use it for charts which are meant to be shared. For example, the name scroll_speed might be changed, which would break all charts using scroll_speed.

On the subject of testing scroll speed changes:

  • Constant scroll_speed=100 and scroll_speed=0 should be well-supported, and any erratic behavior needs to be reported and fixed.
  • Any non-negative scroll_speed and interpolation between them should also be well-supported, but for some large values things might break.
  • Negative scroll speeds are not well-supported now; in particular notes with negative scroll speed in its lifetime might spawn in the middle of the track. This behavior MUST NOT be used as gimmicks as it's prone to be fixed.

@Drewol Drewol merged commit 4062a25 into Drewol:develop Jan 8, 2022
Drewol added a commit that referenced this pull request Jan 9, 2022
Fixed a regression in #515 (objects not visible for certain cases during practice mode)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants