Change osu!taiko's scrolling time range to match stable #26681

vunyunt · 2024-01-23T10:54:13Z

This PR aims to address the time range difference of nomod between current lazer and stable (issue #25919). I'm also planning to base further changes for (classic) mods on this in a future PR, as this provides an easy way to convert between aspect ratio and time range matching stable's algorithm. I'm opening this as a separate PR as it seems to be easier to deal with this way.

To note:

The value of default_time_range is pretty much found by manually comparing with stable (which iirc was also the case for Change Taiko Classic NM, HD, HR and HDHR to better match stable #19604)
The 380px value of ratio_compensator is calculated from 200px (drum display) and 180px (input drum, taken from Define constant input drum width for osu!taiko #26632)
DEFAULT_ASPECT is only used by AspectRatioToTimeRange so it could be moved into there. It's placed on the class for now since it seems to fit there next to MAXIMUM_ASPECT and MINIMUM ASPECT

Comparisons

Note that because of display differences, note overlap and apparent velocity may be different. But time range (amount of notes shown on screen, or the time that a note appears from the right side) should be matching.

These issues should be alleviated by the other existing PRs aiming to match stable's display.

Apologies for the stuttery lazer recordings, I have no idea what's the cause (already using release build) but it doesn't seem to affect timings after notes start playing.

1920x1080 (16:9)

Stable on top and lazer at the bottom

https://youtu.be/R7yaJFNhxQE

1280x1024 (5:4)

Stable on top and lazer at the bottom

https://youtu.be/31CHiWGj1dE

1024x768 (4:3)

Stable on top and lazer at the bottom

https://youtu.be/znIjYMx4Esk

Aspect ratio wider than 16:9

Time range should match each other (aspect ratio limited to <16:9)
Reference on top (lazer 16:9) and wide ratio at the bottom

https://youtu.be/tSo2bz07JGY

Aspect ratio narrower than 5:4

Time range should match each other (aspect ratio limited to >5:4)
Reference on top (lazer 5:4) and narrow ratio at the bottom

https://youtu.be/WaaIMkHn66k

frenzibyte

Preliminary look

frenzibyte · 2024-01-25T20:47:28Z

osu.Game.Rulesets.Taiko/UI/TaikoPlayfieldAdjustmentContainer.cs

+        public const float DEFAULT_ASPECT = 16f / 9f;
        public const float MAXIMUM_ASPECT = 16f / 9f;
        public const float MINIMUM_ASPECT = 5f / 4f;


I think it's better not to introduce terms like "DEFAULT" in public constants in the codebase, I would keep that constant in the helper method instead. Also probably best to make all these constants private, as they're no longer used anywhere outside of the class itself (see other comment).

Makes sense, applied in a46ca14, though do note that the maximum and minimum will need to be adjustable by mods in my next pr (to match stable's wider and mod-variable time range for classic mods)

frenzibyte · 2024-01-25T20:48:53Z

osu.Game.Rulesets.Taiko/UI/DrawableTaikoRuleset.cs

+            float aspectRatio = Math.Clamp(
+                DrawWidth / DrawHeight,
+                TaikoPlayfieldAdjustmentContainer.MINIMUM_ASPECT,
+                TaikoPlayfieldAdjustmentContainer.MAXIMUM_ASPECT);


I would move this inside the helper method, especially since part of the code relies on constants from that class in itself.

Applied in a46ca14. Because it needs access to the draw width and height, I've added a ComputeTimeRange on TaikoPlayfieldAdjustmentContainer too. DrawableTaikoEditorRuleset.ComputeTimeRange seems a bit redundant now, but it's referenced in DrawableTaikoEditorRuleset, so it's kept for now. Do let me know if it would be preferable to just use the one in adjustment container instead.

frenzibyte · 2024-01-25T21:06:19Z

osu.Game.Rulesets.Taiko/UI/TaikoPlayfieldAdjustmentContainer.cs

        public const float MAXIMUM_ASPECT = 16f / 9f;
        public const float MINIMUM_ASPECT = 5f / 4f;

        public readonly IBindable<bool> LockPlayfieldAspectRange = new BindableBool(true);

+        public static double AspectRatioToTimeRange(float aspectRatio)
+        {
+            const float default_time_range = 7000 / TaikoBeatmapConverter.VELOCITY_MULTIPLIER;


It should be explicitly specified that such number was taken purely by visual comparison with stable. Also, can you give further insight as to why this number is being divided by VELOCITY_MULTIPLIER?

Comment added in a46ca14

It being divided by VELOCITY_MULTIPLIER is to signify that it already takes into account the 1.4x speed up of taiko. To quote the current code's comment:

// Stable internally increased the slider velocity of objects by a factor of `VELOCITY_MULTIPLIER`. // To simulate this, we shrink the time range by that factor here. // This, when combined with the rest of the scrolling ruleset machinery (see `MultiplierControlPoint` et al.), // has the effect of increasing each multiplier control point's multiplier by `VELOCITY_MULTIPLIER`, ensuring parity with stable.

Though now that it's a constant, we could just make the default 5000 instead. Or we could include this comment here. Do let me know if one of these would be preferable

frenzibyte · 2024-01-25T21:08:02Z

osu.Game.Rulesets.Taiko/UI/TaikoPlayfieldAdjustmentContainer.cs

+            const float default_time_range = 7000 / TaikoBeatmapConverter.VELOCITY_MULTIPLIER;
+
+            // This is the fraction of the width that should not contribute to time range.
+            const float ratio_compensator = 380f / 1366f;


Would name it non_playable_portion at least.

Renamed in a46ca14

peppy · 2024-01-26T10:00:20Z

I've merged the other changes into this branch. Please make sure that things still look how you expect.

By the way, I'm seeing a slight discrepancy, any idea about this?

CleanShot.2024-01-26.at.10.00.06.mp4

peppy · 2024-01-26T10:03:53Z

Checked with #26616 but still has a discrepancy:

CleanShot.2024-01-26.at.10.03.11.mp4

vunyunt · 2024-01-26T19:18:52Z

I've merged the other changes into this branch. Please make sure that things still look how you expect.

By the way, I'm seeing a slight discrepancy, any idea about this?
CleanShot.2024-01-26.at.10.00.06.mp4

Can you test with default time range changed to 4800 without dividing by velocity multiplier?

I don't think I have access to an exact comparison method (unless the tool shown in the video is public, specifically the ability to synchronize between lazer/stable), but by comparing recordings 4800 seems to match (either the same or better than 5000 or 7000 / 1.4)

4800 as the value could make sense too, since the earlier classic mod states taiko notes scroll at a "constant 100px per 1000ms" and is "referenced to the 480px space", hence it should take 4800ms to scroll through at the default aspect ratio.

peppy · 2024-01-27T14:34:58Z

unless the tool shown in the video is public, specifically the ability to synchronize between lazer/stable

I just synchronised them using pause / cutting edge's rate adjustments (numbers 5-9)

vunyunt · 2024-01-28T06:11:28Z

Ahh, thank you, it is much more precise than recording videos

With that, I've found that the following values matches better:

The 380px value of ratio_compensator is calculated from 200px (drum display) and 180px (input drum, taken from #26632)

It does nullify this particular point I've made in my original post though, I have no idea where 1/3 comes from. I think it's best to just specify that both of these are just taken visually by comparing with stable

Comparisons:

I've switched to the default skin for this, as the skin I was using before seems to not scale identically between the two clients

1280x720

2024-01-28.13-59-24.mp4

1024x768

2024-01-28.14-01-00.mp4

1280x1024

2024-01-28.14-02-40.mp4

frenzibyte · 2024-01-28T06:27:11Z

I'll cross-compare this with stable code and see if I could nail it down to 1:1 match logically.

peppy · 2024-01-29T07:39:40Z

@frenzibyte the hope is to get this in the next release (today). Are you going to get a chance to check this out?

frenzibyte · 2024-01-29T08:01:32Z

I will try my best to look into it today within hours from now.

frenzibyte · 2024-01-29T20:35:08Z

Closing in favour of #26781

Match stable sv, keeping current lazer limits

1731d7b

pull-request-size bot added the size/M label Jan 23, 2024

vunyunt added the ruleset/osu!taiko label Jan 23, 2024

frenzibyte self-requested a review January 25, 2024 11:41

frenzibyte reviewed Jan 25, 2024

View reviewed changes

frenzibyte changed the title ~~Match stable's Taiko time range on nomod~~ Change osu!taiko's scrolling time range to match stable Jan 25, 2024

vunyunt added 2 commits January 26, 2024 15:40

Review changes

a46ca14

Fix code inspect naming issues

d5b08c4

peppy self-requested a review January 26, 2024 09:53

Merge branch 'master' into taiko-sv

62e1481

Finer value adjustments to better match with stable

7da92eb

peppy added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Jan 29, 2024

frenzibyte self-assigned this Jan 29, 2024

frenzibyte mentioned this pull request Jan 29, 2024

Rewrite osu!taiko's time range computation logic to match 1:1 with stable #26781

Merged

frenzibyte closed this Jan 29, 2024

vunyunt mentioned this pull request Feb 15, 2024

Change Taiko Classic HD, HR and HDHR to better match stable #27136

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change osu!taiko's scrolling time range to match stable #26681

Change osu!taiko's scrolling time range to match stable #26681

vunyunt commented Jan 23, 2024

frenzibyte left a comment

frenzibyte Jan 25, 2024

vunyunt Jan 26, 2024

frenzibyte Jan 25, 2024

vunyunt Jan 26, 2024

frenzibyte Jan 25, 2024

vunyunt Jan 26, 2024

frenzibyte Jan 25, 2024

vunyunt Jan 26, 2024

peppy commented Jan 26, 2024

peppy commented Jan 26, 2024

vunyunt commented Jan 26, 2024

peppy commented Jan 27, 2024

vunyunt commented Jan 28, 2024

frenzibyte commented Jan 28, 2024

peppy commented Jan 29, 2024

frenzibyte commented Jan 29, 2024

frenzibyte commented Jan 29, 2024

Change osu!taiko's scrolling time range to match stable #26681

Change osu!taiko's scrolling time range to match stable #26681

Conversation

vunyunt commented Jan 23, 2024

To note:

Comparisons

1920x1080 (16:9)

1280x1024 (5:4)

1024x768 (4:3)

Aspect ratio wider than 16:9

Aspect ratio narrower than 5:4

frenzibyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peppy commented Jan 26, 2024

peppy commented Jan 26, 2024

vunyunt commented Jan 26, 2024

peppy commented Jan 27, 2024

vunyunt commented Jan 28, 2024

Comparisons:

1280x720

1024x768

1280x1024

frenzibyte commented Jan 28, 2024

peppy commented Jan 29, 2024

frenzibyte commented Jan 29, 2024

frenzibyte commented Jan 29, 2024