Skip to content

Add gamepad mode for 5-Fret Guitar #225

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

Merged
merged 8 commits into from
Jan 22, 2025

Conversation

april83c
Copy link

@april83c april83c commented Dec 2, 2024

This PR adds a Gamepad Mode which is accessible as a modifier. It can be customized to strum on both press and release (think Lift Notes or the new gamepad mode updates in Clone Hero 1.1), or to strum only on press (think classic gamepad mode)

2024-12-02.21-18-14-00.00.36.889-00.01.10.018_compressed_to_8mb.mp4

In this next video, pay attention to the keyboard overlay: I'm only pressing half of the notes, because the other half are hit by me releasing/lifting

2024-12-02.21-17-02-00.00.04.288-00.00.22.129_compressed_to_8mb.mp4

(ignore the misses, I have skill issue =w=)

The code in this PR is pretty generic, and will be able to be transplanted into GuitarEngine once hit detection is moved there. The only reason it's not in GuitarEngine right now is that it needs to touch hit detection code.

To-do list:

  • Add a built-in Engine Preset with Gamepad Mode on? Replaced with a modifier
  • Does this cause issues with replays/replay analysis ("inconsistent replay results")?
  • Figure out what to do about releasing from a sustain causing a strum.
    • Currently, I added some code that makes it not strum if you're releasing a fret that's in the mask of a sustain in ActiveSustains, but the problem with this is that releasing after the sustain ends still causes a strum. So, there needs to be some leniency, but I still haven't thought about how to do that.
    • The reason we wouldn't want it to cause a strum is, if you have a note right after a sustain, doing the intuitive thing of releasing the sustain and then pressing again to hit that note might cause an overstrum, and the safest way to hit a note right after a sustain is to just release the sustain and not touch the note (which is pretty lame and IMO shouldn't even be a thing you can do)
  • Figure out open notes (!!!)
  • Disable strumming when Gamepad Mode is enabled? Maybe? Not sure?
  • Retarget to engine-fixes
  • Add overstrum immunity when lifting presses a note, so that if you try to press to hit a note that you already hit by releasing/lifting, you don't get an overstrum
  • Test open chords and open chords during an extended sustain (they should work?)
  • Change default keyboard binds to include DFJKL for frets? (wait, is that in YARG or YARG.Core?)

P.S. please squash commits >.<

Relevant YARG PR: YARC-Official/YARG#929

A build is available here if anyone wants to test: https://github.com/april83c/YARG/releases

@RileyTheFox
Copy link
Collaborator

Hey so thanks for this PR. Turns out we were both working on a Gamepad mode at the same time (I just haven't pushed it anywhere)

So my approach for Gamepad mode was the following:

  1. Duplicate YargFiveFretEngine but called GamepadFiveFretEngine
  2. Remove all strumming logic from it
  3. Create a new modifier called Gamepad Mode and the presence of that modifier determines which engine is instantiated on the YARG client side.

Barring the additional logic in the engine code to create a smooth feeling gamepad mode, the separated code would be a lot simpler than jamming it into the existing engine as you can forego all the complicated strumming logic entirely.

Concerning your tick boxes:

Add a built-in Engine Preset with Gamepad Mode on?

Not a great idea, we'd have to have a Gamepad Mode version of all 3 built in engine presets (Default, Casual, Precision)
A much better idea is to implement it as a modifier or profile option. The presence of the option would change which engine class gets instantiated.

Does this cause issues with replays/replay analysis ("inconsistent replay results")?

Shouldn't do so long as any added timers are accounted for in GenerateQueuedUpdates and there isn't any logic that can get tripped up by a variable update rate.

Figure out what to do about releasing from a sustain causing a strum.

I think removing strum logic entirely is the sanest way to go about it. You can still have "strumming" but without all the mess thats involved trying to handle strumming hopos/taps. Depends a lot on how you want Gamepad mode to work.

Figure out open notes (!!!)

CH has an Open Note button for Gamepad Mode the player can press to hit them, or we can have some basic strumming implementation that can hit opens.

Test if I didn't break anything in normal (non-gamepad) mode, though I shouldn't have

If you implement it as a second engine class, this can't happen :)

If you could also target this PR to the engine-fixes branch. I'm working on a lot of stuff regarding the issues that cause the inconsistent replays, but I don't want to trickle fixes on to master as it means I have to invalidate replays more often. Feel free to discuss how the gamepad mode should work for some of the more nuanced things.

@april83c
Copy link
Author

april83c commented Dec 2, 2024

So my approach for Gamepad mode was the following:

  1. Duplicate YargFiveFretEngine but called GamepadFiveFretEngine

  2. Remove all strumming logic from it

  3. Create a new modifier called Gamepad Mode and the presence of that modifier determines which engine is instantiated on the YARG client side.

I initially thought about doing it as a separate engine, but I decided to do this instead because the logic for making Gamepad Mode work in the existing engine really isn't that complicated, and I didn't want to add a whole other engine that's just the same engine that already exists but with stuff removed, as it would make things harder to update later on (have to update two engines). Also, because if a Festival-like engine is ever added, it could cause confusion, having two unrelated Gamepad engines.

A much better idea is to implement it as a modifier or profile option.

I hadn't considered modifiers. Yeah, that would be a good place to put it.

I think removing strum logic entirely is the sanest way to go about it. You can still have "strumming" but without all the mess thats involved trying to handle strumming hopos/taps. Depends a lot on how you want Gamepad mode to work.

That wouldn't fix the problem, because it's a fundamental problem with being able to hit notes by releasing, not an implementation problem. Also, from my testing, there aren't any issues with hopos/taps.

If you could also target this PR to the engine-fixes branch.

Will do.

@RileyTheFox
Copy link
Collaborator

I didn't want to add a whole other engine that's just the same engine that already exists but with stuff removed, as it would make things harder to update later on (have to update two engines)

That's why overtime I'm working to move more and more shared logic into GuitarEngine/some sort of FiveFretEngine down the line. Moving a lot of functionality into GuitarEngine will help with implementing 6 fret down the line, where really the only main difference is the additional fret to handle and how that affects hit logic. Strumming and other things remain identical.

Also, because if a Festival-like engine is ever added

Never happening - or at least not for a very, very long time. We're not looking to play with the fire that is Epic Games.

That wouldn't fix the problem, because it's a fundamental problem with being able to hit notes by releasing, not an implementation problem.

Can you elaborate on this further? You mention that releasing a sustain causes a strum, so removing strum logic would just result in the sustain dropping when releasing, nothing else. I don't quite follow the problem here.

@april83c
Copy link
Author

april83c commented Dec 2, 2024

Can you elaborate on this further? You mention that releasing a sustain causes a strum, so removing strum logic would just result in the sustain dropping when releasing, nothing else. I don't quite follow the problem here.

The problem is if you have a sustain and then a note right after the sustain, the logical thing is to release (KeyUp) the sustain, then press (KeyDown) the next note. However, you're also supposed to be able to hit notes by releasing (KeyUp) on them. This means that if you do what you intuitively do, you get an overstrum. So I need to add logic to make it so releasing (KeyUp) after a sustain doesn't cause you to hit a note. In this case "strum" and "hit the note" are interchangeable.

That checkbox was only really a note for myself, I know how to do it but I just haven't done it yet

@april83c april83c changed the base branch from master to engine-fixes December 3, 2024 09:40
@april83c
Copy link
Author

april83c commented Dec 3, 2024

Barring the additional logic in the engine code to create a smooth feeling gamepad mode, the separated code would be a lot simpler than jamming it into the existing engine as you can forego all the complicated strumming logic entirely.

I mean, the complicated strumming logic is

  • Leniency for hitting chords so you don't overstrum
  • Not strumming on release if that release is from holding a sustain
  • Leniency for when you press a note that you already hit by releasing

None of these actually have anything to do with strumming, they have to do with hitting notes. They all would need to be implemented anyways even if it was a separate engine with no strumming. It's not in any way simpler than jamming it into the existing engine like I'm doing here.

So I don't really see the point making a whole separate engine when the logic would be exactly the same amount of complicated there, and it would be even more complicated because, well, it'd be a whole new engine with completely different logic (because no strumming) that needs to be written and maintained/updated.

@RileyTheFox
Copy link
Collaborator

So I don't really see the point making a whole separate engine when the logic would be exactly the same amount of complicated there, and it would be even more complicated because, well, it'd be a whole new engine with completely different logic (because no strumming) that needs to be written and maintained/updated.

My point is that jamming it into the existing engine can actually make it harder to maintain. You've now got to juggle both gameplay modes in the same logic, being careful not to break one when changing the other. Engine code is already pretty precise on how it needs to be written so that it feels good to play.

With the plans to pull out as much common logic into base classes, the only thing the Standard/Gamepad classes would be responsible for is their own unique logic. Then modifying one won't potentially cause problems with the other.

Your original comment had this:

Test if I didn't break anything in normal (non-gamepad) mode, though I shouldn't have

If they were separated, this would literally be impossible and you wouldn't even need to test it 🙂

@april83c
Copy link
Author

april83c commented Dec 3, 2024

My point is that jamming it into the existing engine can actually make it harder to maintain.

I understand your point. I just disagree with it because of how simple the Gamepad Mode logic is.

I think it's way simpler to just keep it in the same engine instead of making an entirely separate engine.

I think the way I wrote it, most of the time there won't be Gamepad Mode issues when making changes to the engine, because of how simple it is — it's just making the frets strum, and handling some quirks that come with that.

Also because of that, I think having it in the normal engine itself and not a separate class is good, because then once hit detection is moved out into GuitarEngine, this code can be moved there too, making it work for both 5-fret and (future) 6-fret.

If they were separated, this would literally be impossible and you wouldn't even need to test it 🙂

I already don't need to test it. When I wrote that, I wrote it for myself and myself only, just to double check. And I wrote it expecting any possible issues to be "I forgot a if (IsGamepadMode)", and not some actually hard to figure out/subtle bug, because there isn't really any logic that could cause anything like that.

@april83c april83c marked this pull request as ready for review December 9, 2024 23:10
@RileyTheFox RileyTheFox self-requested a review December 10, 2024 01:25
@RileyTheFox
Copy link
Collaborator

Just writing that I haven't forgotten about this, I'm just super busy currently. Will hopefully be able to get round to this in a couple of weeks.

@april83c
Copy link
Author

april83c commented Jan 3, 2025

Just writing that I haven't forgotten about this, I'm just super busy currently. Will hopefully be able to get round to this in a couple of weeks.

No worries, it's all good!!

@RileyTheFox RileyTheFox changed the base branch from engine-fixes to gamepad-mode January 22, 2025 15:05
@RileyTheFox RileyTheFox merged commit 460c5b1 into YARC-Official:gamepad-mode Jan 22, 2025
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