robreuss / VirtualGameController

Software-based game controllers for iOS, tvOS, OS X and watchOS in Swift 4.2.
MIT License
462 stars 44 forks source link

Button release not firing for Peripheral_iOS #18

Closed toddyocum closed 5 years ago

toddyocum commented 8 years ago

I have a SceneKit based app on Mac OS X that is using Virtual Game Controller as a Central and am using the Peripheral iOS app distributed with Virtual Game Controller to control it.

In my app I have registered for buttonA button press change's, similar to the DemoBot's example.

DemoBots/GameControllerInputSource.swift

The only differences is I am using it as an extendedGamePad, so I have something like this:

        // GCExtendedGamepad trigger handlers.
        if let extendedGamepad = gameController.extendedGamepad {
            extendedGamepad.buttonA.pressedChangedHandler = attackHandler
        }

And for now just a simple 'pressed' detection.

let attackHandler: GCControllerButtonValueChangedHandler = { [unowned self] button, _, pressed in
            print("Button pressed: \(pressed)")
            if pressed {
                self.shooting = true
            }
            else {
                self.shooting = false
            }
        }

Once everything is running and I press the 'A' button on my iPhone running Peripheral iOS , I observe my attackHandler ( a GCControllerButtonValueChangedHandler) getting called with 'pressed' set to 'true'.

However, when I release the 'A' button, I don't get a similar call to my handler with 'pressed' set to 'false'.

So assuming I have things coded correctly, I traced the issue to VgcController.swift line 2219

I change the '&&' to '||' and observed the expected behavior for both press and release.

If you need a better replication I could probably try it with the DemoBots app.

robreuss commented 8 years ago

I'll try to reproduce the issue today.

With your fix, does the pressedChangedHandler get called only twice, once for the press and once for the release? That's what's supposed to happen to be consistent with the GCController behavior, and it looks like your fix should accomplish that.

toddyocum commented 8 years ago

For my limited testing of about 10 minutes last night at midnite, yep, thats the behavior I saw. :)

What was really confusing me was if I tapped the button enough repeatedly/rapidly, I would sometimes get a release in the pressedChangedHandler. Note, this was before the fix, when I was trying to understand the issue. So maybe some kind of race issue? I couldn't see how it was possible, and then stopped investigating when I figured out what seemed to be a fix, so just a 'fwiw'.

Thanks for taking a look at it.

toddyocum commented 8 years ago

Quick Update, I got DemoBots and integrated Virtual Game Controller and replicated the problem.

Unfortunately, my possible fix doesn't work as needed, pressedChangeHandler is getting called multiple times.

In my app it didn't really matter, so I didn't notice until I revisited to see.

robreuss commented 8 years ago

Sorry I didn't get to this today - will tomorrow. It seems like I didn't really implement the logic for releasing the press - I'll fix that up.

robreuss commented 8 years ago

Just pushed a fix. Cleaned up and clarified the logic for managing button state. Both handlers valueChangedHandler and pressedChangedHandler should be delivering a correct pressed value. Inspection of the pressed property of the button in other contexts should work as well.

Let me know how it goes!

toddyocum commented 8 years ago

It does work correctly if I press and release quickly.

But if I hold down the button and wiggle my finger, I get multiple pressed=true events, even though I'm just holding the button, so seems to be something weird going on still. And I also managed to get multiple pressed=false events too.

Pressed: true Pressed: true Pressed: true Pressed: true Pressed: true Pressed: true Pressed: false Pressed: false Pressed: false Pressed: false Pressed: false

robreuss commented 8 years ago

Should be fixed now.

toddyocum commented 8 years ago

Seems pretty close, no more repeat press events while just pressing.

The only way I can get repeats in a row now is if I tap the button really fast, and I'm not sure if maybe thats expected or not given that your framework has to shuttle events over a network? Are events guaranteed to stream in order from peripheral to bridge.

So for example, I see something like this now when tapping fast.

Pressed: true Pressed: false Pressed: true Pressed: false Pressed: true Pressed: false Pressed: true Pressed: false Pressed: true Pressed: false Pressed: false Pressed: false Pressed: true Pressed: false

I also see the occasional latency spike, mostly on release, seems to go as high as 300 - 500 ms or so if I had to guess.

For my app's purposes, the current behavior is going to work pretty good, since I don't need fast tapping and that kind of occasional latency will be tolerable, I think.

robreuss commented 8 years ago

Network communication is stream-based and should be first in, first out.

I see the same problem when rapidly tapping. Hardware controllers work fine, unsurprisingly because it is the GCController framework itself that calls the handlers.

I disabled the lines that call dispatch_async that executes the handler based on the handlerQueue value the developer can set and it seems to fix the problem:

            if buttonPressedChanged {
                if let pressedChangedHandler = vgcPressedChangedHandler {
                    //dispatch_async((vgcGameController?.handlerQueue)!) {
                        pressedChangedHandler(self, newValue, self.vgcButtonPressed)
                    //}
                }
            }

            if let valueHandler = vgcValueChangedHandler {
                //dispatch_async((vgcGameController?.handlerQueue)!) {
                    valueHandler(self, self.element.value.floatValue, self.vgcButtonPressed)
                //}
            }

My current approach to dealing with the handlerQueue is to default the value to dispatch_get_main_queue, and to run the handler on that - but typically, that will already be the queue that we're executing on. So dispatching may just creating overhead and could be causing things to get out of order? Alternatively, I could only dispatch if handlerQueue if it's non-nil. It would be a big refactor but if this is potentially causing latency and sequencing issues, no choice.