search

libretro-mirrors / beetle-saturn-libretro

Standalone port of Mednafen Saturn to the libretro API.
GNU General Public License v2.0
62 stars 42 forks source link

Touchscreen lightgun support #148

Closed yoshisuga closed 5 years ago

yoshisuga commented 5 years ago

Support lightgun input using touchscreen. Added a core option to set the gun input mode (defaults to Lightgun).

For multi touch inputs to work, RetroArch needs to be updated to support RETRO_DEVICE_ID_POINTER_COUNT - the number of touches in the current input. This is the PR in the RetroArch repo: https://github.com/libretro/RetroArch/pull/8959

hiddenasbestos commented 5 years ago

Shouldn't touchscreen input feed into lightgun at the front-end layer, not as part of the core?

yoshisuga commented 5 years ago

I can certainly see the argument for that, but touchscreen was considered to be a different input device (RETRO_DEVICE_POINTER), and i was following the convention of handling lightgun input using different devices here, in a different core: https://github.com/libretro/libretro-fceumm/blob/master/src/drivers/libretro/libretro.c#L1384

hiddenasbestos commented 5 years ago

I commented on this already and I feel it's important to reply again to formally step back from this, rather than leaving a negative opinion floating.

I still feel that the work you're doing here would benefit RetroArch (and therefore all cores) more and if you* worked on a lightgun driver for that which was built around a touch screen as the physical input source (rather than using the mouse) there wouldn't need to be any changes to this core, or the PS1 core or the others that use the lightgun API now, or in future.

But right now the framework for that is not there AFAIK, so there's not a short term fix.

So to be clear: I'm not objecting to you contributing to this core, but I see a ton of what I see as front-end features get added into cores (or already there) and it's a shame things aren't as clean as they could be! -- so I commented out of passionate frustration more than anything :)

In my opinion, only Nintendo DS and Wii U emulators should be using the RETRO_DEVICE_POINTER, everything else using it for light guns is mis-using this API and should stop it (this was because before I came along and fixed the lightgun API, the pointer API was the only way to get an absolute position needed for gun games.)

inactive123 commented 5 years ago

Can you come up with a counter proposal then in API form along with a working prototype?

At the end of the day, what needs to be delivered is simple:

1 - It needs to have a working implementation in RetroArch at the same time as the feature is integrated into the API. 2 - It needs to be working with at least one core. 3 - It cannot break ABI.

Apart from that, I'm fine with anything. If you can come up with something that does the job the same way yoshisuga's work does, and if @yoshisuga agrees with it, then both sides are happy and we can both move on with our lives.

hiddenasbestos commented 5 years ago

I wouldn't imagine the API / ABI needs to change at all, it's just a matter of where the +/- 0x3fff comes from. That's in higher level RetroArch territory, rather than LibRetro API, and not really something I can commit time to working on.

My post above was more of an explanation of where I'm coming from, but since I can't work on anything better I'd also say might as well go with this for now (and a short term gain) and remove it later when the front-end gets better.

Bit worrying that other cores appear to be setting a precedence when actually they're doing the "wrong" thing too. I have my opinion and I've shared it, but I'm just an armchair commentator in this so probably I should have kept it to myself if I can't walk the walk as well :)

inactive123 commented 5 years ago

Please just give us the necessary info we need then to improve it according to what you think should be done. Otherwise we're just beating around the bush here and not getting anywhere.

@yoshisuga could do something with the input you provide then. State exactly what needs changing, how it could be done, and it might be able to be looked into. Just state specifically the specs so there can be no confusion.

hiddenasbestos commented 5 years ago

Ok, I feel like I might be frustrating you, sorry about that!

The main thing is cores using RETRO_DEVICE_POINTER for lightgun inputs is "wrong" based on the conceptual nature of what the 'pointer' device class is supposed to represent (it should really be called retro_device_touchscreen, this conversation would be a lot clearer!!) - existing cores that use the pointer API .. shouldn't .. IMHO.

This change request is, well I'm not sure what the point of it is other than to use a different api to drive the lightgun data into the emulation part of the core. I suspect it comes about from lightgun not being implemented very well in RetroArch on certain platforms, rather than this being a logical thing to do for the core - but then again, it's just a little bloat, probably not worth worrying about (<-- not sarcastic)

What I'd change in RA, if I could, would be:

That's my back of a napkin outline, to answer your question. It's not really anything to do with cores, it's just about how the front end acquires the data.

But I can't spend any more time on this, I've not got the resources to implement big changes to RA, so I'm just trying to provide my experience to guide it where I can. Probably in an annoying way, sorry!

inactive123 commented 5 years ago

Yeah, in fact, it is a bit annoying, since even this list of ideas here is way too impractical to implement right now, especially not if you don't want to make the PR yourself. It needs to be far more realistic and limited to the kind of code @yoshisuga is implementing.

So you say it's 'wrong' what we're doing but there is no real sense of 'what' is so wrong about it or why it is so terrible even. Or more importantly, why it even matters.

Again, if you don't want to invest the time into coming up with a suitable alternative to what @yoshisuga is doing, we're going to go with that instead. We need to keep this project moving along and it needs to keep improving, and we can't keep getting filibustered on these things. It's fine to point out criticism and to say this or that is wrong, but I do expect for that person then to provide the necessary code or pointers to make it better then, so that we can reconcile both sides here and find a middle ground. I gave the window of opportunity for things here to be changed, and instead people don't want to invest the time into it to make the changes according to your liking. Nothing I can do beyond that then, and we go with this instead. There was a window of opportunity here, there is still after this for all I am concerned, but a fully working RetroArch implementation for every libretro API addition is absolutely a 100% necessity, and I will never, ever sway from that point. Nothing enters the libretro API officially without RetroArch being able to support it Day 1. People can judge that all they want, but that is my executive decision and has been for some time.

inactive123 commented 5 years ago

I mean no harm BTW in stating this, just being very honest, that is all. I have no issues with you and I think your PRs have been great, and definitely value your opinion and feedback, which is why I ask. I am just disappointed I guess that the opportunity to find a middle ground here in so far as this implementation is concerned is not seized upon. I can't see how to make both sides happy then. I'd still ideally like to have a solution that both of you guys would be happy with. But I need you then to get involved in this PR, otherwise we cannot get there.

Is the issue here that this causes complications for your frontend or what is the deal here? Is there actually a serious pressing issue here?

hiddenasbestos commented 5 years ago

No problem, I appreciate your reply. Feedback without an attached PR puts you in a difficult situation. I don't mean to stand in the way of this getting merged in. Thanks for your time.

inactive123 commented 5 years ago

OK, we'll go with this then for now. I'll be happy to act on any feedback/future PRs you have for further improving this down the line. Let me know and we can get @yoshisuga in then too to comment on it.

yoshisuga commented 5 years ago

Just catching up with the discussion here. I appreciate the feedback. I understand the want to decouple the core and input logic, and we can create another layer of abstraction on top of the input driver to translate touch inputs to lightgun inputs, but even after doing so - we'd probably still have to make adjustments in the core for touch input. The reality in the implementation shows that we need to tweak things for touch input, as was the case for this core - touch was too sensitive for the input that i had to make cycle-level adjustments to get it to work as expected. Not to mention it would require pretty extensive changes across the board in the RetroArch code, including several existing input drivers.

I feel like my implementation is just an additional branching logic in the lightgun handling, and the logic should be pretty straightforward to look at, not really introducing any complexity, and benefits many existing platforms.