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

Use input method depending on chart keymode #705

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Chazoshtare
Copy link
Contributor

@Chazoshtare Chazoshtare commented Jun 25, 2022

Fixes issue: #678

Previously, it was only possible to play charts using the input method picked as music select input option. This PR introduces a change that will select appropriate input method depending on the type of chart being played.

Additionally, previous logic of using an incrementing int with modulo operation, to determine which song select input option was chosen was refactored into using an already existing enum with all possible options, what possibly makes adding another options in the future much easier.

I discovered that song select option also determines which keys can be bound in key binding screen, and that state only changed when user got back to song select screen and again to key configuration. I moved the part of code that was setting this input method to class responsible for key binding screen, and it changes instantly when user chooses a different song select input option. So overall, it didn't solve the underlying issue, (bindings are still limited by option chosen), but at least it's somewhat better as it changes in the binding screen. Once this PR is merged, I'll create a separate issue that explains this problem in detail and maybe try to work on it too.

Part of this PR just removes unneeded whitespace, but it's split in separate commits. Main commit that represents the main logic change can be viewed here: dc8d23e

@Chazoshtare Chazoshtare force-pushed the fix/678-use-bound-controls-for-chart-keymode branch from 8bc0e2e to 8ecf91e Compare April 29, 2023 11:49
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.

1 participant