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

FlxTouch and FlxButton pressed being missed #1370

Open
JoeCreates opened this issue Nov 10, 2014 · 21 comments
Open

FlxTouch and FlxButton pressed being missed #1370

JoeCreates opened this issue Nov 10, 2014 · 21 comments

Comments

@JoeCreates
Copy link
Member

JoeCreates commented Nov 10, 2014

This seems to be most noticeable when using touch, for example, when touching FlxButtons. Button pressed are often missed, making the game feel unresponsive. How long you touch a button for makes a difference. It is only quick presses that get missed.

I have tried adding the following traces to FlxButton#onUpEventListener:

#if !FLX_NO_MOUSE
    private function onUpEventListener(_):Void
    {
        trace("ONUPLISTENER");
        if (visible && exists && active && status == FlxButton.PRESSED)
        {
            trace("handled");
            onUpHandler();
        }
    }
#end

With the above, a quick press on a button causes "ONUPLISTENER" to be traces, but not "handled". The reason is that the status is not yet PRESSED. If the button has not been updated before the touch has been released, it will likely be missed as the state is incorrect.

It looks like this could be due to the fact that Inputs can be pressed and released in a single update, and yet the input cannot simultaneously be in the states JUST_PRESSED and JUST_RELEASED. The rapid release is meaning that the JUST_PRESSED state is being missed.

Also, is it possible that an input is going to be updated before the button? If so the input could move from JUST_RELEASED to RELEASED before the button has had time to check if the input was just released, and thus the onUp callback will be skipped.

@Gama11 Gama11 added the Bug label Jun 23, 2015
@Gama11
Copy link
Member

Gama11 commented Jan 16, 2016

The fix for #1686 might have helped with this (2c9a545). Not really sure how to test it.

@JoeCreates
Copy link
Member Author

I don't think so. That seems to be an unrelated issue. @Tw1ddle and I fixed this on a private branch of flixel, but completely changes the states inputs can have, and so it is incompatible with Flixel's input recording.

It has to be possible for a button to be both justPressed and justReleased on the same frame.

This is a major issue for mobile devices (ours and other's mobile games have been rejected from app store for having unresponsive buttons, although submitting multiple times eventually gets it through).

I would suggest that the existing system is simply broken, but it is unfortunate that the recording is dependent on it. Perhaps we could merge our fixed version of FlxInput in and make it optional to use the old system for flxrecord?

@Gama11
Copy link
Member

Gama11 commented Apr 4, 2016

I'm not sure it's only recordings that depend on this..

@JoeCreates
Copy link
Member Author

@Gama11 Here's how it looks in our new version: https://gist.github.com/JoeCreates/aba33862ddfb75d3071b9e5d219878df

justPressedQueueFalse and justReleasedQueueFalse ensure the justPressed and justReleased states last for a frame.

We kept the public interface as close as possible to the existing interface. For the most part, existing classes extending FlxInput or implementing IFlxInput will work the same. The reason FlxRecord breaks is that it assumes an input only has one state in any given frame, and modifying this will break existing recordings.

@sruloart
Copy link
Contributor

sruloart commented Apr 4, 2016

The VCR isn't even relevant for mobile (touches aren't being logged AFAIK, right?), so, this solution can be blocked within an #if FLX_TOUCH (thus not being a breaking change, simply a fix for a serious usability issue).

P.S And the VCR is just a glorified keylogger for the mouse and keyboard, I have no idea why it's even on the Flixel's core (probably for (pre)historic reasons). If something breaks that let it break I say :D

@Gama11
Copy link
Member

Gama11 commented Apr 4, 2016

Mouse input works just fine on mobile.

@sruloart
Copy link
Contributor

sruloart commented Apr 4, 2016

Not if we break it :D

And more specifically, does the recording system work on mobile?

and also, this: #1739

@JoeCreates
Copy link
Member Author

The issues arent actually mobile specific, its just more likely to manifest
on mobile due to low frame rates on old devices and the fact that touches
tend to happen faster than mouse clicks.

The primary way we tested this was on desktop, and we did it by setting the
game to update at a very low frame rate.

You can, if you really try on a game with a lower frame rate, have the
issue appear when using the mouse. You just have to click extra quick, e.g.
by tapping the mouse button.

If it is provided as an option, I think it would make sense to have the
fixed version be the standard and default. A new recording system could be
written, but to support older games with old recordings there would still
need to be the option to use the old version.
On 4 Apr 2016 22:49, "Sruloart" [email protected] wrote:

Not if we break it :D

And more specifically, does the recording system work on mobile?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1370 (comment)

@IBwWG
Copy link
Contributor

IBwWG commented Apr 20, 2016

The old recording system is broken anyway, pending my PR.

Wouldn't it be most sensible to have pressed be a boolean all by itself, indicating the plain current state (which, in the case of a quick tap, would be false), then justPressed and justReleased would be two more independent booleans. This allows all possible combinations of reality, and a few non-sense that are not possible to get around:

p   jP  jR
t   t   t   doesn't happen
t   t   f   started pressing this frame
t   f   t   doesn't happen
t   f   f   started pressing before this frame
f   t   t   quick tap this frame
f   t   f   doesn't happen
f   f   t   stopped pressing this frame
f   f   f   stopped pressing before this frame or was never pressing

The "doesn't happen" cases should be handled accordingly (trace a warning indicating a bug?)

The other cases allow us to solve the current issue.

The lack of a released boolean removes even more impossible cases that would be possible with the gist (and maybe are currently), where something can be both pressed and released simultaneously. So we can implement it for backwards compatibility as a getter function returning !pressed.

As for the currently-broken-has-been-for-a-while-anyway recorder, it could be made to be backwards-compatible if we just use higher-numbered enums for each of the above combinations (except f-f-f, which can be 0) going forward, and map the old "2" to t-t-f in the table above.

@IBwWG
Copy link
Contributor

IBwWG commented Apr 20, 2016

In fact, maybe it would make sense to have an enum like this (for the five above combos that do happen):

justPressed
stillPressed
justPressedAndReleased
justReleased
notPressed

This could hold the actual state and be backwards-compatible with the current recorder and most current things via getter functions.

So depending on the incoming events for a given frame:

notPressed either becomes justPressed or justPressedAndReleased, or stays the same
justPressed becomes either stillPressed or justReleased
stillPressed either becomes justReleased or stays the same
justPressedAndReleased and justReleased both have the same options as notPressed (because you could have multiple single-frame taps in a row; when done tapping you revert to notPressed)

Then the backwards-compatible getter functions would be:

justPressed: justPressed || justPressedAndReleased
pressed: justPressed || stillPressed
justReleased: justReleased || justPressedAndReleased
released: notPressed || justReleased || justPressedAndReleased

@JoeCreates
Copy link
Member Author

JoeCreates commented Apr 20, 2016 via email

@IBwWG
Copy link
Contributor

IBwWG commented Apr 20, 2016

OK, fair enough. You did put up a gist, I referred to it (without a link)...or do you mean something else?

I guess then I would still propose 3 bools instead of 4 as an improvement, so that at least pressed and released are mutually exclusive.

Although, on second thought--the situation you presented is a good one to consider, and what about beyond that? What if the user taps and releases twice or three times within a frame? Especially in a gaming context this might be desirable to capture properly. In that case, I would go back to the 5-enum, and have an additional var completeTapsThisFrame:Int that keeps track of how many complete taps there were. So in the press-release-press example you gave, the state would be justPressed but completeTapsThisFrame would be 1. But applications would need to be pretty aware of this...backwards compatibility might not give you what you expect if you just compile a working desktop game for phones and encounter this for the first time. Isn't there already a multiple-gesture API for touch in flixel? Oh, that makes it even more complicated...how do you know what was two separate taps completed within a frame, and what was a multitouch gesture?

Maybe flixel should just forward down/up events on directly, playing them in order?

That, or it's just a known byproduct of having a low-FPS app...there's only so much you can/should try to do within a frame if your frames are already lagging. (I mean, it entirely depends on the application. If processing additional taps doesn't add too much more lag, then it's not like you're making a bad situation worse; but in some games you very well could be.)

@IBwWG
Copy link
Contributor

IBwWG commented Apr 20, 2016

Regarding your initial issue, though, I wonder if it's something with FlxButton in particular. To me that class always seemed a bit strange, since it uses OpenFL event handling but only for mouseups, and then also messes with FlxInput updating. (I had to do something similar in the VCR code mostly because of this messing--otherwise played back mouseclicks do not function at all on FlxButtons.) There is also the FlxMouseEventManager class...have you tried it out in context? If so, and if it's any better than FlxButton, maybe we could rewrite FlxButton to use FlxMouseEventManager instead of the way it currently does things.

@Tw1ddle
Copy link

Tw1ddle commented Apr 20, 2016

@IBwWG You could queue events up and forward them on, or handle them immediately. That way you'd be able to process every event and this kind of problem wouldn't occur. However this boolean flags fix is a relatively simple change and fixes the most common problems.

About mouse ups on FlxButton, it was necessary to defer calls to onUpHandler until the updateStatus(input:IFlxInput) method to get it working how I wanted (I don't recall the exact problem I had though... 😄 ).

@JoeCreates
Copy link
Member Author

@Tw1ddle I just saw the "hack" you added. Was this actually still necessary after the changes to FlxInput?

@Tw1ddle
Copy link

Tw1ddle commented Apr 27, 2016

@JoeCreates I couldn't remember, so I made this demo... Try switching between the flixel dev submodule and the other branch in project.xml: https://github.com/Tw1ddle/FlixelInputBug

On current dev flixel, you reproduce the basic problem by spam clicking the button and noting any difference between the number of OpenFL mouseup events and the number times the button is triggered.

On the branch with the added *queue flags in FlxInput and the button mouseup deferral check, any single click over the course of a frame is guaranteed to be handled. Spam clicking still means events are dropped, but that's just how it works, since this implementation doesn't queue events up.

And if you remove the hacky extra fix I put in FlxButton, fast clicks on the button made within a single frame seem to get missed: Tw1ddle@53e7002

So yeah. This problem will crop up when slow devices will run games at a poor framerate, so mobile mainly.

@JoeCreates
Copy link
Member Author

Just spoke to @Tw1ddle on skype. I have some information as to why this issue exists in FlxButton and also a suggestion for how to fix it.

The reason FlxButton is a special case is that apparently on flash certain operations can only be performed if the user initiated it (such as opening a new window https://github.com/Tw1ddle/flixel/blob/62bf66104f29ca1c3d514306015cb04ab97c322f/flixel/ui/FlxButton.hx#L518). To allow FlxButtons to do this extra code was added to make FlxButton use an onUpEventListener and perform the buttons actions after that. The problem with this is that it still checked flixel's PRESSED state on the button to ensure that the button had been clicked before the up event happened. Flixel may skip the PRESSED state if the input happens quickly at a slow framerate, so the previous condition in onUpEventListener would fail.

I don't know if this security issue mentioned is still a thing. If it is not, then this special button case should be removes. If it is still an issue, however, then a anDownEventListener could be added which sets a new "flash state" (as opposed to the flixel state) of the button to being down. This would be checked by onUpEventListener instead of flixel's PRESSED.

@Gama11
Copy link
Member

Gama11 commented Apr 27, 2016

Flash security restrictions definitely haven't changed.

@JoeCreates
Copy link
Member Author

@Gama11 Yeah, Sam confirmed this.

@mastef
Copy link

mastef commented Dec 30, 2016

I think we're running into this on Androids and iPads on buttons that extend FlxButton. Quick presses on the button get ignored at a 30-40 fps framerate ( or when there's frame drops ).

What's the recommended solution currently? Any plans to fix it?

@mastef
Copy link

mastef commented Jan 5, 2017

Spoke with @Tw1ddle in Slack about this, for anyone else looking for solutions these 2 patches should be of help :

FlxInput patch :
Tw1ddle@87d00c6
FlxButton hack :
Tw1ddle@53e7002#diff-75008da6c068aef2396448261140bc16

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

6 participants