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

Bug fixes, can press several buttons now. We'll still probably need to tweak this. #38

Merged
merged 13 commits into from
Aug 20, 2019

Conversation

medape
Copy link
Contributor

@medape medape commented Aug 7, 2019

I've tried to separate my edits in different commits, but some of them may have some undocumented code tweaks or bug fix.

Short story:

Graphical fixes

At some point I broke base.css, so the joysticks were painted far away from their correct positions, and there were multiple double definitions. I think I corrected all of these bugs.

I also made positive to include preserveAspectRatio="none" in joystick.svg and dp.svg so there won't be any mismatch between the background image and the actual control. If we desperately want any of them to be square, we can define this.shape = 'square' in the pertinent JS object.

All positions and hitboxes are recalculated now when resizing the window. This won't have much use within the Yoke app, but part of the code was already there, and it's very useful for debugging.

Some changes in variable names

Controls now have multiple references: instead of just accessing Joypad._controls as an array, you can now use Joypad._controls.byMnemonicID and Joypad._controls.byNumID. (I really need to learn to use better names when coding.)

I needed to be able to use both kinds of references: the numeric one is still used for reporting the state from the Yoke app, and the alphanumeric one is used for...

findNeighbourhood()

Please rename this as well, if you have a better name.

The only way to be able to press several buttons with just one finger was for them to share events. I thought that reading events directly from div#joypad and tracking the finger positions for every button in every touch event would be too slow and too complicated, so I developed this function. It can be tweaked to avoid some repeated calculations, but it's very fast as it's written.

This function finds the mnemonic for the control in the CSS, reads every control that's directly touching it (on edges or corners), and outputs every for them as an array sorted and without duplicates. For example, using findNeighbourhood on b1 and this layout:

b1 b1 b2
b1 b1 .
b4 .  j1

will return ['j1', 'b1', 'b2', 'b4', '.']. Yes, it also outputs itself and the empty squares.

This function is called ahead of time and the controls are referenced in Button.neighbourhood (or AnalogButton.neighbourhood). The non-button controls are filtered, but every button lists itself in neighbourhood because...

Overlapping buttons

The buttons now lay a transparent div .buttonhitbox, over the button, and make it slightly bigger. A different method, but for the user it works more or less like the D-pad. The only difference is that the padding of this hitbox is based on absolute pixels instead of fractions of a square. I should probably tweak this, but I wasn't sure how, and decided to leave it to later commits.

Every time one of these hitboxes registers a touch event, it's shared with every button in neighbourhood. Every button checks the finger coordinates against its own hitbox, and then the button that received the original touch event runs updateState() once, so the computer will register the button presses as simultaneous.

Analog buttons

I decided to make some changes to this to make it more intuitive.

If you're using force detection, it uses that, as usual. Every button you press with the same finger reports the same force.

If you're not, it's based in distance to the center, so every pressed button will report different "forces" depending on your finger position.

The real change here is that the engine doesn't check if the button is wide or long beforehand. Analog buttons now check the position on both the X and Y axis, and report the lower one. The only way to press a button with full force when using this system is to touch it in the dead center.

Maybe we could add a dead zone around the center. As the calculated force will be truncated to [0.000001, 0.999999] anyways, multiplying it times 1.05 or some other number should be enough.

D-Pad accepts now more than one finger, updateDebugLabel() now works if and only if there is a debug label, and event handlers for knobs are simpler now.
Positions and measurements are correctly recalculated upon window resize, and iteration over touch events on D-Pad is now faster.
Fixed beta angle calculation. Motion and orientation events no longer owned by arbitrary motion tile. joypad._controls can now be read .byMnemonicID and .byNumID.
This is a helper function to share events among overlapping buttons. Also simplified the code for truncate()
D-pad background now strecthes to cover the whole area, and touch events are shared among neighbouring buttons.
I hadn't tried AnalogButton.prototype.processTouchesForce. Now it checks for the position of the finger, and takes advantage of the force calibration.
@pzmarzly
Copy link
Collaborator

pzmarzly commented Aug 8, 2019

Thanks a lot for this PR.

I really need to learn to use better names when coding.

So do I.

findNeighbourhood()
Please rename this as well, if you have a better name.

Maybe findNeighbors()? BTW US vs UK spelling is not that much of an issue, though in README we used US one: behavior.

Maybe we could add a dead zone around the center. As the calculated force will be truncated to [0.000001, 0.999999] anyways, multiplying it times 1.05 or some other number should be enough.

I think that's a good idea.

Neighbourbood changed to Neighbors, default control shaped moved to prototype, getBoundingClientRect() repeats one calculation in exchange for writing less repetitive code.
Analog buttons now have a dead zone when translating position to force. Knobs behave as before, but their initial state is halfway between the minimum and the maximum. (This seems like a wise general precaution.)
Reduced number of quadrants in knob, and programmed buttons to vibrate every time a button is pressed through processTouches(), not only the hitbox that originally received the touch event.
@medape
Copy link
Contributor Author

medape commented Aug 10, 2019

With this, your suggestions are implemented and a couple of shortcomings in my code are fixed, too. Please check.

I can't think of anything else I want to add to this PR. Do you have anything?

@pzmarzly
Copy link
Collaborator

I can't think of anything else I want to add to this PR. Do you have anything?

I can only think of some minor things:

var ANALOGBUTTON_DEADZONE_CONSTANT = 1.10;

Could we have it named ANALOG_BUTTON_DEADZONE_CONSTANT (extra underscore)?

By the way, why is state called _state, but the old state oldState (no _)? Would it be possible to get rid of all _variables (_state, _mask, _locking etc.)?

@medape
Copy link
Contributor Author

medape commented Aug 12, 2019

Could we have it named ANALOG_BUTTON_DEADZONE_CONSTANT (extra underscore)?

Sure.

By the way, why is state called _state, but the old state oldState (no _)? Would it be possible to get rid of all _variables (_state, _mask, _locking etc.)?

No reason, actually. When I first started editing the JavaScript, I noticed some of the variables had an underscore. I assumed at the time it was reserved either for "important" or "private" variables and didn't think much of it.

So of course it would be possible. Much easier, really. We just need to choose different names for the function .prototype.state() and the variables ._state. Maybe .prototype.stateToInt() and .state?

@pzmarzly
Copy link
Collaborator

I assumed at the time it was reserved either for "important" or "private" variables and didn't think much of it.

Perhaps it was, ages ago.

We just need to choose different names for the function .prototype.state() and the variables ._state. Maybe .prototype.stateToInt() and .state?

IMO variable should be called .state. Here are some ideas of mine for the function name: report(), reportState(), getState(), serialize()

and vJoy's driver load in particular, by updating the whole joypad all at once instead of control by control.
@medape
Copy link
Contributor Author

medape commented Aug 16, 2019

Okay, I think this is all. I removed the underscores, and used reportState().

I'm going to be somewhat absent for some days (I'll check this project often, but I don't think I'll find the time to write new code), so I decided to push a couple unrelated changes I was working on.

vJoy now updates every control simulataneously

Well, almost. Yoke processes every control one by one, but sends all the information to vJoy at once (using the function UpdateVJD directly).

The method we've been using until now (setAxis and setButton) is the one recommended by vJoy's developers, as it's easier and guaranteed to work after a change in API. However, the API has been stable for years, and they admit that every time we call a high-level function, vJoy calls UpdateVJD once. This way is quicker and doesn't overload vJoy as much.

Reduced socket read buffer to 64 bytes

I've also reduced the socket read buffer because the latency in WIndows was horrible and I couldn't find any other way. 64 bytes will be more than enough for any layout when we switch to binary protocol anyways.

Lag in Windows still hovers 100 ms (I haven't looked for a way to measure it yet) but Yoke in Linux has become even quicker, if anything.

I suspect upgrading to Android System WebView 76 has broken the motion controls

onDeviceOrientation events are never fired now. I even tried previous versions from this repo, and as far as I recall they used to work, but not now.

We'll need to check this.

@pzmarzly
Copy link
Collaborator

onDeviceOrientation events are never fired now

You mean that motion controls don't work at all? They work for me, but I'm using LineageOS webview, so perhaps you are right with this not being yoke issue. It's troubling nonetheless. I'll open another issue.

Awesome job with input delay reduction, by the way.

@pzmarzly
Copy link
Collaborator

@medape please see #39 .

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.

2 participants