Change Taiko Classic NM, HD, HR and HDHR to better match stable #19604
Conversation
|
I feel a deep urge to restate my concerns about having mods influence the way that other mods function. I have brought this up several times before, but in short, the argument goes that allowing this sets a bad precedent and invites exponential complexity both to mod logic and to evaluation of beatmap difficulty and player performance. I know that stable did this, but I'm also saying that I think it was a bad idea and it should not be repeated again. |
|
@bdach I don't disagree, but are you suggesting that we need to break parity with stable to avoid this? The original goal of "classic" mod was to create an experience for players that are allergic to change. |
|
Honestly, in the worst case? Absolutely. But before that, I would explore options as to whether it is possible to change the logic of the mods affected by this change such that they always work like they did on stable, and examine what that means for the new lazer gameplay mechanics. As in, get the changes in, but remove the part that makes these changes conditional on classic being active, and see how broken the end result is when classic isn't active. |
|
Just to keep information parity between discord and here I guess: I could go along with this as long as whatever is implemented is constrained to classic mod and it is not allowed to have normal mods change the function of other mods outside of "classic" contexts. Ideally all of the classic interdependency brokenness would also be implemented in |
|
This PR does try to limit changes to As for making mods always work like they do in stable, for HD and HR at least it might be possible, if there is a way to ensure HR changes to the playfield always override HD. HR might not even have to pass additional parameters to HD as it seems like that can be achieved by extending the playfield beyond the display, which I did try before opening this PR but couldn't get it to scale well. |
| @@ -28,7 +29,13 @@ public class TaikoModHidden : ModHidden, IApplicableToDrawableRuleset<TaikoHitOb | |||
| /// How long hitobjects take to fade out, in terms of the scrolling length. | |||
| /// Range: [0, 1] | |||
| /// </summary> | |||
| private const float fade_out_duration = 0.375f; | |||
| public readonly Bindable<float> FadeOutDuration = new Bindable<float>(0.375f); | |||
There was a problem hiding this comment.
For safety you can use BindableFloat and then set MinValue and MaxValue to the intended range (for this and the bindable below).
There was a problem hiding this comment.
Makes sense, applied in 8000ae6
As a side note, IBindable.cs in osu-framework seems to have a potential minor typo at line 18 and 91:
|
I haven't done a detailed pass on this, but overall the code quality and structure is looking about how I'd expect it to be. |
osu.Game.Rulesets.Taiko/UI/TaikoPlayfieldAdjustmentContainer.cs
Outdated
Show resolved
Hide resolved
|
Code-wise I'm fine with this diff now. I loosely visually checked the end result against stable, and it seems indistinguishable to me (being a taiko layperson), but I haven't gone to confirm all of the numerical detail against stable source. Let's just say I'm personally willing to let players playtest and see how it goes. |
|
Have pushed a trivial merge conflict resolution (was in the |
|
@smoogipoo can you take a quick look at this and make sure you don't have any strong feelings against the way the cross-mod mutation is done here? I'm not 100% about it, but we should probably address this PR before it gets too far behind. |
|
@vunyunt sorry that your PR got stuck in a really bad state, pending on review from out end that got forgotten. i just want to ping you and see if you're still around and/or willing to contribute. I'm finally getting a chance to revisit this (based on further reports of things being wrong, like #25919) and working through your changes here, which do still look like what we want. Unfortunately a lot has changed since this PR got forgotten about and bringing them across via a merge resolution is... quite high effort. If you're still around and wiling, I am happy to delegate a reimplementation of this to you, else I'll see what I can do. |
Hi, yes I am available for this. I assume this means it would probably be easier to reimplement this on the latest codebase instead of trying to bring the current implementation up to date? |
Either is fine! But we'd be looking to get the changes in ASAP as we are looking to bring the next stage of scoring online soon. Also of note, we may want to match the display resolution in non-classic mod too, but we can address that after things are brought up-to-date as required. |
Alright, I'll take a look at the up to date repo first, most likely I'll be reimplementing it using this branch as a reference. By matching display resolution do you mean matching stable's SV? (in reference to #25919) |
Alright, I have started working on it earlier today but haven't had too much changes yet, so it shouldn't be too much of an issue. I'll avoid making too much change before those then, and will try to come up with something functional within this week |
Changes include:
I can't quite get HDHR to scale to different resolutions identically to stable, hence I went with close enough. The videos below is one commit behind and HDHR had been further scaled closer to stable since, but there isn't a big difference.
Videos showing gameplay vs. stable (ordered by NM -> HD -> HR -> HDHR):
1920x480
1280x720
800x600
Code wise this is achieved by defining
IApplicableToTaikoClassic, which lets mods register themselves toTaikoModClassic.TaikoModClassicthen handles the scalings accordingly.TaikoPlayfieldAdjustmentContainerhas also been expanded to handle different type of visual scaling methods.