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

Gauging Interest in Controller Support and Potential Pull Request #677

Open
Myvar opened this issue Apr 2, 2024 · 6 comments
Open

Gauging Interest in Controller Support and Potential Pull Request #677

Myvar opened this issue Apr 2, 2024 · 6 comments

Comments

@Myvar
Copy link

Myvar commented Apr 2, 2024

Hello,

I hope this message finds you well. As someone who suffers from repetitive strain injuries, I prefer to minimize my keyboard usage when not programming and instead use a controller for gaming. With that in mind, I have managed to implement working controller support for Brogue by expanding the SDL platform code.

Before investing further effort into refining this implementation, I wanted to reach out to the community and the development team to gauge interest in this feature. Would there be others who would benefit from having controller support in Brogue? Additionally, I wanted to inquire if you would be open to a pull request to upstream controller support into the main repository.

It's important to note that my current implementation is a bit hacky and not yet suitable for a pull request. One fundamental change I made, which is rather messy, involves the platform code receiving information about the game's UI state. This allows the buttons to change their actions based on the displayed screen. For example, I have the A button mapped to "explore," but when in the inventory, it changes to "apply" or "equip" the selected item, reverting back to "explore" when the inventory is closed.

The challenge lies in the fact that Brogue was not originally designed with this architecture in mind, and my current implementation relies on some messy hacks to make it work. To contribute this feature properly, a decent amount of refactoring would be required to avoid it being a "hack."

Before investing the extra effort to refine the code and make it suitable for a pull request, I wanted to gauge the interest and thoughts of the community. Is controller support something that others would find valuable? If so, I would be happy to put in the work to clean up the implementation and submit a pull request.

Thank you for considering this idea, and I look forward to hearing your thoughts and feedback.

@zenzombie
Copy link
Collaborator

Sounds great to me. Interested in thoughts from @tmewett and @flend.

@flend
Copy link
Collaborator

flend commented Apr 8, 2024

I'm supportive. There are other examples where'controller support' scenarios for brogue would be useful, e.g. the iOS/Android port or the DPAD in web-brogue http://brogue.roguelikelike.com/ (which currently has non-contextual buttons).
I think passing in a enum of context into platform_nextKeyOrMouseEvent would be reasonable and doing the translation there into the event depending on context (perhaps with a global helper function). This would imply adding the context param to nextBrogueEvent and any others.
The constants used in the game (e.g. DISCOVERIES_KEY) are mostly semantic so it seems reasonable to map onto them (rather than refactor them completely). We'd still embed these into recordings etc. so recordings would not remember the input context/controller used.

@btaylor84
Copy link

I'm the current dev that has iOS brogue on the Apple store, sadly it's not current with the other versions. Part of the reason I've been a little daunted in updating is that I have to resolve the updates with the alterations in the primary engine code to allow hooks for the tasks iOS needs to perform. A refactor that makes those hooks explicit in the main engine code would make this problem much less a hassle for new versions. So, this controller code might be a way forward for that.

@Myvar
Copy link
Author

Myvar commented Apr 8, 2024

If we can design the refactor to be useful for more people, I'm all for that. Maybe we can break this down into two PRs/Tickets: one that introduces the state information into the platform code, and one that is the controller support. That way, we separate the two concerns, and we can get input from more people like @btaylor84 to add the things they need.

@tmewett
Copy link
Owner

tmewett commented Apr 15, 2024

Maybe we can break this down into two PRs/Tickets: one that introduces the state information into the platform code, and one that is the controller support

That sounds like a good idea!

Do you have any ideas to mind about how you'd like the platform API to look? Is your current code somewhere public?

@Myvar
Copy link
Author

Myvar commented Apr 15, 2024

Ahoy, @tmewett!

Is your current code somewhere public?

The code is not public, i just download a zip of the repo hacked in controller support and started playing the game with my Frankenstein build, i can make a proper fork and rework it into that once we have decided how to move forward.

But i would be happy to give you the code i wrote in a zip or something or email you a diff path etc, the code is real bad, but i think it would be a good conversation starting point on how to shape things. (Here is a gist of the largest changes)

Do you have any ideas to mind about how you'd like the platform API to look?

I know very little about Brogue's design from what i can see the ui's are implemented as while loops and do not really have a central separated place to live, i don't think there is any need to change this the Intermediate mode UI is doing its job and its doing it well but we might introduce entry and exit functions in the same way ImGui has ImGui::Begin and ImGui::End and then add those into every ui loop to capture state changes, the begin function can accept relevant metadata arguments, but there might be cases where all metadata is not available just before the start of the loop. so some kind of patch/update function would make sense to change the state in the loop it self as user input is handled.

I have spent less than a hand full of hours in this code base so my suggestions are probably silly and misinformed, and i welcome corrections from others.

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

No branches or pull requests

5 participants