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

Streamline keyboard input #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Streamline keyboard input #71

wants to merge 1 commit into from

Conversation

andres-asm
Copy link
Contributor

@andres-asm andres-asm commented Jan 3, 2018

Following the conversation here:
libretro/libretro-cap32#23

We decided to try to establish some conventions for some undefined behavior in the API so both core devs and frontend devs can play by the same rules.

Goals:

Define how KBD / Mouse should work in cores

  • Physical KBD / Mouse always should work (in cores that require KBD and Mouse)
  • When the ports are set to RETRO_DEVICE_KEYBOARD gamepad input on that port is disabled so a frontend can dedicate that port to KeyMapper input (if implemented)
  • RETRO_DEVICE_NONE should always be honored (meaning no inputs are passed)

This is just an idea and we hope we can get some traction so computer cores can become more popular / user friendly.

Following the conversation here:
libretro/libretro-cap32#23

We decided to try to establish some conventions for some undefined behavior in the API so both core devs and frontend devs can play by the same rules

Goals:

Define how KBD / Mouse should work in cores

- Physical KBD / Mouse always should work (in cores that require KBD and Mouse)
- When the ports are set to RETRO_DEVICE_KEYBOARD gamepad input on that port is disabled so a frontend can dedicate that port to KeyMapper input (if implemented)
- RETRO_DEVICE_NONE should always be honored (meaning no inputs are passed)

This is just an idea and we hope we can get some traction so computer cores can become more popular / user friendly
@andres-asm
Copy link
Contributor Author

About the last part:

RETRO_DEVICE_NONE should always be honored (meaning no inputs are passed)

In this PR I'm handling this in the core, but I guess the frontend could handle it too to avoid polling when it's not required

libretro/libretro-cap32#23 (comment)

@garbear
Copy link
Contributor

garbear commented Jan 3, 2018

Can you give examples of what the calls to retro_set_controller_port_device() look like for the following cases?

  • Only a keyboard
  • Keyboard and two connected joysticks
  • Keyboard and two disconnected joysticks

@andres-asm
Copy link
Contributor Author

You mean the frontend side?

@garbear
Copy link
Contributor

garbear commented Jan 3, 2018

Yes

@andres-asm
Copy link
Contributor Author

andres-asm commented Jan 3, 2018

Well I'm not really sure, I mean the frontend doesn't really care about specifics, basically iterate over your configured number of maximum users setting the frontend ports one by one.

https://github.com/libretro/RetroArch/blob/master/command.c#L978

Following what we just agreed upon these cases:

  • Only a keyboard
  • Keyboard and two disconnected joysticks

Would be impossible, you wanted keyboard to be always active, that's what we're doing here and now we're basically establishing ONE condition were it's not active (all ports set to none)

@andres-asm
Copy link
Contributor Author

Maybe we can change the writing here:

When the ports are set to RETRO_DEVICE_KEYBOARD gamepad input on that port is disabled so a frontend can dedicate that port to KeyMapper input (if implemented)

To

When the ports are set to RETRO_DEVICE_KEYBOARD gamepad input on that port is disabled so a frontend can dedicate that port to KeyMapper input (if implemented), if one or more ports are set to RETRO_DEVICE_KEYBOARD the physical keyboard is also active.

@garbear
Copy link
Contributor

garbear commented Jan 3, 2018

I think we need an out-of-band method to indicate keyboard presence. Using joystick ports fails if there are no joystick ports.

@garbear
Copy link
Contributor

garbear commented Jan 3, 2018

If you use a core option please include this in the libretro API. Frontends shouldn't have to have knowledge of 60 cores and their options to disable a keyboard.

@andres-asm
Copy link
Contributor Author

Yeah I removed that comment, I guess we need an env callback to explicitly enable / disable the global keyboard.

@garbear
Copy link
Contributor

garbear commented Jan 3, 2018

Right, then when the keyboard is enabled, it can be queried using RETRO_DEVICE_KEYBOARD on port 0, and joysticks (if any) can still be queried using RETRO_DEVICE_JOYPAD and their port. How's that sound?

@andres-asm
Copy link
Contributor Author

andres-asm commented Jan 3, 2018

Well, with these changes basically physical keyboard is not attached to any port which was the initial goal.
What we're missing is a way to explicitly enable the keyboard if no ports are attached.

Changes to the API are not my forte, I'm only here because I wanted the keymapper so I'm just trying to achieve what you want without major changes to RA or the API.

I think we can leave the API change for later, and basically no way to explicitly enable the keyboard for now?

Otherwise we need @twinaphex and @Alcaro to chime in.

@inactive123
Copy link
Contributor

inactive123 commented Jan 3, 2018

Changes to the libretro API will not be made lightly and will only be done after heavy deliberation, and I see that as falling outside of the scope of this particular PR anyway.

We will not fall into the trap of so many other projects where the ABI / API is just broken carelessly just to support new features, it has to be done in an intelligent way to ensure all fronts remain on the same page and confidence in the API isn't lost.

Even the VFS PR (after it has finally been merged) is still giving us some problems here and there, and I kinda wish that we had taken even longer before we finally got that merged, since obviously not enough testing was performed still and we were not careful enough. If this had been ffmpeg, we'd have broken API about 20/30 times a year already and everybody would have to endlessly update to the latest ffmpeg API. I think this is kinda the bane of multimedia players' existence, and probably the reason why Kodi and other projects just hardfork ffmpeg entirely. We want to avoid that deadtrap entirely for libretro. Basically, the extreme vetting process that goes into any changes to libretro is to avoid another ffmpeg scenario from emerging, where nobody can rely on the main API to remain stable and every project that depends on it (VLC/Kodi/PPSSPP/etc) just starts hardforking away in order to have anything stable.

So yes, any API additions will not only be carefully vetted to ensure backwards compatibility and to ensure no interfaces are broken in the process, but I think in this case it also falls outside the scope of this particular PR.

I don't see anything wrong with this current PR. I will leave it up to @fr500 as to whether or not he wants me to merge this or not, I have no real opinion on it currently.

@andres-asm
Copy link
Contributor Author

Ok I'll be checking this again on the weekend and I'll try to bring dosbox, px68k up to speed with this

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.

3 participants