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

DeviceInputSystem: Add configure/removeEvents to be called by DeviceSourceManager #11959

Merged
merged 17 commits into from
Feb 17, 2022
Merged

DeviceInputSystem: Add configure/removeEvents to be called by DeviceSourceManager #11959

merged 17 commits into from
Feb 17, 2022

Conversation

PolygonalSun
Copy link
Contributor

This PR contains a few fixes. First, the contract for DeviceInputSystem has been changed to add configureEvents and removeEvents as public functions. These functions will now also be called by the DeviceSourceManager, in order to enable and disable event handling. The DeviceSourceManager will also have enableEvents and disableEvents. These functions will call configure and remove respectively. The enable and disableEvents functions should only be called by the InputManager but can tehcnically be called by other classes or the user. Finally, the default max number of touch points if navigator.maxTouchPoints doesn't exist is now 2 to support the usage of device that may be interpreted as touch but aren't touch. 2 was also picked because we only really support up to 2 finger gestures.

src/DeviceInput/InputDevices/webDeviceInputSystem.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/webDeviceInputSystem.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/webDeviceInputSystem.ts Outdated Show resolved Hide resolved
src/Inputs/scene.inputManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed offline live. Comments below.

src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/Inputs/scene.inputManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
@RaananW
Copy link
Member

RaananW commented Feb 16, 2022

@bghgary - I leave it to you to resolve the conversations and approve

src/Engines/Extensions/engine.views.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/webDeviceInputSystem.ts Outdated Show resolved Hide resolved
src/DeviceInput/InputDevices/webDeviceInputSystem.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two small nits

src/DeviceInput/InputDevices/deviceSourceManager.ts Outdated Show resolved Hide resolved
src/Inputs/scene.inputManager.ts Outdated Show resolved Hide resolved
@PolygonalSun PolygonalSun merged commit a08fd40 into BabylonJS:master Feb 17, 2022
@PolygonalSun PolygonalSun deleted the PolygonalSun/create-remove-events branch August 9, 2022 17:47
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