Add "Alternate" mod

[LeNitrous]

Closes #3773 and supersedes #7953. This mod forces the player to alternate between keys for osu and taiko rulesets. It injects a keybind handler to the drawable ruleset and intercepts and reads input allowing the mod to allow or disallow certain actions.

Considerations:

• Key states are reset after break times as usually people tend to forget what last key they pressed causing unintended misses.
• In taiko, big drum hits can be hit with 2 keys. I dislike the idea of a mod preventing basic gameplay mechanics such as this. However, if you hit one with 1 key, you must alternate with another key for the next drum hit.

Possible considerations:

• I'd like to keep the key counters as is to find out why they missed in replays or recordings.

Issues:

• In osu, sliders still judge tracking thus giving score to ticks, repeats, and ends. This may be because it listens for a button down input instead of listening to an InputAction. This is intentional to allow switching between keys while tracking inside a slider.
• In taiko, strong hit objects may have not reset the input state yet causing other hit objects inside their hit window to possibly miss (as we don't know what key was last used)

Prerequisites: #10472

[bdach]

The kkdd vs kddk discussion from the previous PR seems to be unaddressed, although I'm not sure how important that is. The transform usage seems somewhat strange as well; I'd probably construct a list of times upon which the button state should be reset, and then iterate over it during gameplay in Update() (first frame that crosses the given time causes a reset and moves the iterator pointing at the next time instance that should cause a reset forward one item).

[enoslayd]

I know this comment might get deleted but....

What if instead of failing, it will become miss instead (or add this as an option?)

[bdach]

There is some feedback already to the user in that the taiko drum parts won't light up if the player fails to alternate. Misses will also still register, but not immediately upon mistakenly not alternating - they work as if you just didn't press the key (whether they should is arguable, as they add the ability to recover by alternating again).

[enoslayd]

Does it apply to osu ruleset too?

[bdach]

Yes, with the same caveats.

[LeNitrous]

Currently I just based off of "alternating" as alternating between keys. Also using transforms is overkill too. I like the idea of storing timestamps in an array and is much more safer as it accounts for rewinding. However I don't think I have a way to access the gameplay clock's current time.

@enoslayd It currently blocks your input thus making you miss. You can just add Sudden Death or Perfect if you want the option to fail.

[bdach]

Isn't it just Time.Current? As you're adding the interceptor to the drawable ruleset hierarchy, it should be under the gameplay clock.

[enoslayd]

Some suggestions (delete if this comment not necessary):

1. In my opinion, It should ignore swells (spinners). You just generally bash them
2. After spinners, i think the keys should return to normal. What i mean is that they will be reset like when entering a break (maybe also consider drumrolls (sliders) too)

Edit: iirc, auto on taiko does full alternate already

[LeNitrous]

Applied bdach's suggestion of iterating over the current time instead of using a lot of transforms. Plus this should properly track strong hit objects now as it doesn't rely on their hit windows (it checks whether the hit object is a strong object and resets the input states to guarantee we can use either key in the next object).

As for the suggestions by enoslayd and the concern about kkdd and kddk in the previous PR, it would be up for debate by other people for now.

[LeNitrous]

I've wrapped up the correct way on handling strong hit objects now, -snip-
OK I just needed someone to explain the kddk and kkdd issue earlier.

[LeNitrous]

OK after some considerations this should handle cases with strong hit objects (big drum hits) better. It's difficult to disambiguate between 1 key press and 2 key presses as we can't anticipate what the player will do on the first frame. As a solution, this is how it handles those cases:

1. If the next hit object is a strong hit object, allow any key to be used. This allows 2 keys to be used.
2. If the strong hit object is hit with 2 keys, any key can be used for the next hit object.
3. However, if it is hit with 1 key, that key can't be used for the next hit object.

This should avoid cheesing but not in all cases due to the complexity of the problem as stated above.

Next, I added two modes as stated in the first PR. These are KDDK and KKDD respectively, referring to the key bindings Kat and Don respectively. The names are temporary for the two modes aren't very friendly yet as they're taiko player jargons. If there are better names, it should be replaced.

[LeNitrous]

@Waidxd changes as suggested have been applied. Please check whether this behavior is correct.

[Waidxd]

@LeNitrous behaviors works perfectly now however I ran into a separate issue regarding big dons and big kats.

In the following scenario:
small don --- big don --- small don

If I were to do as PlayStyle.KDDK or as PlayStyle.KKDD
TaikoAction.LeftCentre --- (TaikoAction.LeftCentre + TaikoAction.RightCentre) --- TaikoAction.LeftCentre

I will end up missing the 2nd small don

but in the scenario of:
big don --- big don --- big don

If I do
(TaikoAction.LeftCentre + TaikoAction.RightCentre) -- (TaikoAction.LeftCentre + TaikoAction.RightCentre) -- (TaikoAction.LeftCentre + TaikoAction.RightCentre)

This seems to work fine... so I am not really too sure what is causing the issue yet or how to resolve it

[bdach]

Strong hits should probably reset the last hand used? Not sure what is the expectation here.

[LeNitrous]

It's intended whenever the next hit object is a strong, any keys can be used regardless if the last key is the same. This is to not block the mechanic of requiring 2 keys for a full score on strong hit objects.

[Waidxd]

so the more common approach among full alternators from both kddk and ddkk players given the following scenario (capital d for large don):
d --- D --- d --- D --- d
l --- l+r --- l --- l+r --- l
or
r --- l+r --- r --- l+r --- r
essentially we reset after every large note

The following is the less common approach and what current implementation enforces, its not wrong by any means.
d --- D --- d --- D --- d
l --- l+r --- r --- l+r --- l
or
r --- l+r --- l --- l+r --- r
There is very little to no stamina saved from the second approach I mentioned which is why most full alternators tend to go with
the first approach as it is a lot easier to not mess up

I guess the first approach could later added as an option in the future

[LeNitrous]

I forgot to mention that when the player successfully uses two key on a strong hit object, the key states will be reset allowing for any key to be used on the next. However if the player uses only one key, the key states won't be reset and checks will be performed on the next.

@Waidxd for your case, the key states are always reset before the next hit object's judgement if it is a strong and should allow both scenarios to be accepted input.

[Waidxd]

Can confirm after testing manually that the latest change by @LeNitrous fixed/allowed this approach.

so the more common approach among full alternators from both kddk and ddkk players given the following scenario (capital d for large don):
d --- D --- d --- D --- d
l --- l+r --- l --- l+r --- l
or
r --- l+r --- r --- l+r --- r
essentially we reset after every large note

I tried both play styles again and I can't really find any bugs/flaws with the current implementation after going through a number of different scenarios. I think its good to go!

[LeNitrous]

Moved the mod to Difficulty Increase as it plays similar to Perfect and Sudden Death where it also adds a layer of difficulty to the gameplay. Also added a keyboard icon to it.

[LeNitrous]

I've based the "first non-miss" hit result by checking the highest combo achieved since that would look more simpler compared to calling ScoreProcessor.GetStatistic() multiple times for each non-miss HitResult since combo is based on successful hit results.

[peppy]

I'm abstaining from further review of this PR until inline comments and a code quality pass is done. Currently it reads worse than osu-stable code, especially TaikoModAlternate.

Go through your code, extract methods where possible (and name them logically) and add inline comments wherever the code doesn't immediately explain what is being done.

[peppy]

code quality needs work

[peppy]

why?

[LeNitrous]

Highest combo is based on the number of non-miss hits. It is possible to do ScoreProcessor.GetStatistic() multiple times for each hit result type, this is much simpler.

[bdach]

I can't parse this comment. What does this aim to do exactly? Not process inputs until the first user hit? I'd actually think using just Combo would be more correct, to allow recovery from any position after a miss.

Edit: Never mind, I think I get it now. It's supposed to ignore everything until first hit.

[peppy]

what do these mean?

[LeNitrous]

I don't have proper names for them since I'm not really a taiko player. Though they're taiko jargons for keybind arrangements. KDDK is Rim-Centre-Centre-Rim and KKDD is Rim-Rim-Centre-Centre (or vice versa).

[bdach]

FWIW, this is what I have locally and intend to push in here when I'm done:

        public enum Playstyle
        {
            /// <summary>
            /// Each hand has a rim and a centre button, so alternating is done by changing hands.
            /// Also known in community vernacular as "kddk".
            /// </summary>
            [Description(@"Alternate hands (""kddk"")")]
            AlternateHands,

            /// <summary>
            /// One hand has both rim buttons and the other both centre buttons.
            /// Alternating is done using fingers only.
            /// Also known in community vernacular as "kkdd".
            /// </summary>
            [Description(@"Alternate fingers (""kkdd"")")]
            AlternateFingers
        }

Still a little arcane, but maybe a little better?

[LeNitrous]

I'm still not confident whether I refactored enough of the code. Feedback is appreciated.

[bdach]

Not good enough still, sorry. The state management is (at least for me) easily the worst part of this PR - the worst parts of inheritance show here. Some parts of the logic/state are in the base mod, some in the ruleset-specific ones. The taiko logic also needs less intertwining between execution paths (ideally, you'd have a straight switch across hitobject types, and then handle each cleanly in a separate method).

I'll give a proper attempt at taking over this PR tomorrow.

[bdach]

I've made decent headway into straightening this out but haven't managed to finish yet. Preview branch here. It has some wonkiness still (consecutive strong hits aren't behaving as expected) but I think it looks way better than what's here.

I tried writing tests for this but they're way too involved and verbose even in the simple cases, and even when writing headless (tried simulating the state changes by exposing methods as protected). I'm not even sure the taiko one is the correct direction; it might honestly be easier to take the beatmap, run an algorithm over it to determine actions permitted for each hitobject and then read from that as we go in the mod itself.

[peppy]

fwiw, i don't think the user should need to track the "expected" key for a specific object and weakly agree with how it's done at the moment (adapting to the user).

if we're to do it as a preprocess step there 100% needs to be visual feedback on the objects or HUD as to which key is next. as it stands, visual feedback would still be appreciated but isn't imperative.

[bdach]

The way I was thinking of wouldn't differ much from what's here in practice - it wouldn't be just "press this one and only expected key for this object", I'd be doing some fudging to make sure the user can pick up alternating again from any given point in time. But hopefully that's not a bridge I'll have to cross if I can get the current version working correctly.

[bdach]

I fiddled with my version some more today but I'm still not entirely happy with how it works, so I'm putting this on the backburner due to relatively low impact/priority.

[LeNitrous]

if we're to do it as a preprocess step there 100% needs to be visual feedback on the objects or HUD as to which key is next. as it stands, visual feedback would still be appreciated but isn't imperative.

I've taken the time to review the code back and see what I can do about it and in this scenario. Its true there should be some visual feedback whenever the user presses the wrong key and what better place would it be is on somewhere the player always has their eyes on which is usually either the cursor or the next object.

For standard, we can make it flash red on the cursor whenever the next input is invalid. For taiko, the area where the judgement line is. I'll have to implement it to see it in practice still.

I'm not even sure the taiko one is the correct direction; it might honestly be easier to take the beatmap, run an algorithm over it to determine actions permitted for each hit object and then read from that as we go in the mod itself.

While the standard implementation is relatively simple enough given there are only two keys to keep track of and only one type of input expected to be received, there are readability problems with taiko's end given how complex the requirements are. Even I had issues writing tests before. I'm not too sure on how this can be further improved.

Here are my findings about taiko's input:

• The strong hit objects have the hit object itself and one child hit object that its only purpose is to detect whether a two-key press was done within a set amount of time which is usually short. This implementation was also rewritten in the mod as the hit object itself doesn't expose this.
• The rim and drum are simple and handled similarly like standard's however they expect a specific key press.
• Drum rolls (yellow sliders) accept both rim and drum hits but must be hit in time consistently. However this case still needs to be handled similarly to the point above.
• Swells are ignored and can be mashed.

[ribbanya]

@bdach

I fiddled with my version some more today but I'm still not entirely happy with how it works, so I'm putting this on the backburner due to relatively low impact/priority.

I know this was a while ago so maybe you don't remember but what did you find unsatisfactory about your branch?

As a strictly alternating player who also wants to learn the osu codebase and contribute if I have time, I'd like to take a crack at this.

[bdach]

I don't recall anymore, sorry. I think it just didn't work 100% correctly even after my attempted changes.

[mk56-spn]

i think imma nuke the taiko part and PR this for STD alone

[peppy]

It’s called “osu!” not “STD”.

[LeNitrous]

I think taiko's issue was code quality and not to mention tries to accommodate all the nuances of alternating in taiko alone. I don't want it to be wasted effort honestly but if its really the case then I can close this PR and make a new one branching from this for the osu! part to be implemented so the original branch wont get affected.

[mk56-spn]

Up to you it's your code ig. Dunno what peppy or bdach will think. Eitherway something seems broken with on pressed and on released in the main file so yeah . Probably caused by updates to things around it

[LeNitrous]

Stemming off from #16479, I'll look into splitting the PR into two parts separating the osu! version of the mod from the taiko version to help reduce the number of discussions from forming.

[StayBlue]

Can confirm that this mod basically works outside of the box on osu! (standard) (see below for issue and fix). I have not tested the taiko implementation.

Adding onto the comment posted by @mk56-spn, I simply needed to update OnPressed and OnReleased to use KeyBindingPressEvent<TAction> and KeyBindingReleaseEvent<TAction> respectively. The reason being is that IKeyBindingHandler was updated back in July. Other than that, seemingly nothing else needs to be touched for the mod to work.

One thing that maybe wasn't actively considered when making this mod is the problem of keys still being registered by the key overlay even when they aren't supposed to be. This is not really an issue, but just thought it was worth mentioning. Just realized this was mentioned already and I completely missed it when I was searching for whether it was already mentioned or not.

[LeNitrous]

Superseded by #16701