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

Implement basic touch support #334

Closed
wants to merge 3 commits into from
Closed

Implement basic touch support #334

wants to merge 3 commits into from

Conversation

TomBebb
Copy link
Contributor

@TomBebb TomBebb commented Aug 24, 2020

Hi guys, first proper pull request!

This implements touch motion and finger pressing using winit API - see bug #300

I couldn't decide on finger vs touch in struct names, I am happy to refactor code if anyone has better ideas for names

Is it worth storing last finger positions so we can give delta in finger motion events like we do in mouse motion events?

@Moxinilian Moxinilian added C-Feature A new feature, making something new possible A-Input Player input via keyboard, mouse, gamepad, and more labels Aug 24, 2020
@RobDavenport
Copy link
Contributor

IMO I think going with struct names based on "Touch" makes more sense than "Finger." Because touch input can include things like stylus and pen tablets, it would be strange to call them fingers.

@zenMaya
Copy link

zenMaya commented Aug 26, 2020

I don't know, but could you use #59 Axis API for the finger position? Or is it too different?

@naithar naithar mentioned this pull request Aug 30, 2020
@MichaelHills
Copy link
Contributor

+1 on calling it touch. Btw I didn't realise this PR exists and duplicated some efforts here #87 (comment) Perhaps it may be of interest. I actually implemented an alternative to Input to keep track of current touch state including distance moved from start position. I think we should land raw events first and we can build plugins on top with various abstractions and see what sticks.

I don't know, but could you use #59 Axis API for the finger position? Or is it too different?

Perhaps is what I did in my link closer to what you're looking for? Rather than a raw event stream.

app.resources.get_mut::<Events<TouchFingerInput>>().unwrap();
touch_finger_input_events.send(converters::convert_touch_input(
ElementState::Released,
&touch,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should be mapping Cancelled to Released. The Input abstraction as it stands doesn't map well to touch. I think either we extend Input or create a different abstraction for touch... I think I personally would prefer the latter.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with this. We should mirror the winit TouchPhase enum in the bevy events.

// This system prints messages when you use the touchscreen
fn touch_system(finger_input: Res<Input<Finger>>) {
if finger_input.pressed(Finger(0)) {
println!("finger 0 pressed");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure this works on all platforms? For example on iOS I was getting huge numbers, not 0 and 1.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we should assume that a "touch id" is an opaque identifier.

@@ -192,6 +193,64 @@ pub fn winit_runner(mut app: App) {
});
}
},
WindowEvent::Touch(
Copy link
Member

Choose a reason for hiding this comment

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

I think its much more legible to compress these into a single match arm, then have a second match inside:

WindowEvent::Touch(touch) => match touch.phase {
    TouchPhase::Started => {
        let mut touch_finger_input_events =
            app.resources.get_mut::<Events<TouchFingerInput>>().unwrap();
        touch_finger_input_events.send(converters::convert_touch_input(
            ElementState::Pressed,
            &touch,
        ));
    }
    TouchPhase::Ended => {
        let mut touch_finger_input_events =
            app.resources.get_mut::<Events<TouchFingerInput>>().unwrap();
        touch_finger_input_events.send(converters::convert_touch_input(
            ElementState::Released,
            &touch,
        ));
    }
    TouchPhase::Cancelled => {
        let mut touch_finger_input_events =
            app.resources.get_mut::<Events<TouchFingerInput>>().unwrap();
        touch_finger_input_events.send(converters::convert_touch_input(
            ElementState::Released,
            &touch,
        ));
    }
    TouchPhase::Moved => {
        let mut touch_finger_moved_events =
            app.resources.get_mut::<Events<TouchMotion>>().unwrap();
        touch_finger_moved_events.send(TouchMotion {
            finger: Finger(touch.id),
            position: Vec2::new(touch.location.x as f32, touch.location.y as f32),
        });
    }
},

app.resources.get_mut::<Events<TouchFingerInput>>().unwrap();
touch_finger_input_events.send(converters::convert_touch_input(
ElementState::Released,
&touch,
Copy link
Member

Choose a reason for hiding this comment

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

I agree with this. We should mirror the winit TouchPhase enum in the bevy events.

/// A finger on a touch screen device
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
pub struct Finger(pub u64);
Copy link
Member

Choose a reason for hiding this comment

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

I also think I would prefer it if we didn't call this finger. I think TouchId is a more generic term

// This system prints messages when you use the touchscreen
fn touch_system(finger_input: Res<Input<Finger>>) {
if finger_input.pressed(Finger(0)) {
println!("finger 0 pressed");
Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we should assume that a "touch id" is an opaque identifier.

}

/// Updates the Input<Finger> resource with the latest TouchFingerInput events
pub fn touch_finger_input_system(
Copy link
Member

Choose a reason for hiding this comment

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

As nice as this api is to consume, I think we should remove Input<Finger> for now. Touch events might need their own abstraction because they have more state to track and TouchId might be unstable / hard to know ahead of time.

@MichaelHills
Copy link
Contributor

@TomBebb are you planning on updating this? Otherwise myself or others could help push this along.

Is it worth storing last finger positions so we can give delta in finger motion events like we do in mouse motion events?

I think so yes. In my iOS bevy project I store start and current positions and use distance from the start position for virtual joysticks. If the joystick is moved past some "max" distance, the joystick itself then starts to move around the screen.

@naithar naithar mentioned this pull request Oct 17, 2020
2 tasks
@cart
Copy link
Member

cart commented Oct 20, 2020

Closed in favor of #696

@cart cart closed this Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants