Split PointerId into distinct id and PointerKind fields #17808

alice-i-cecile · 2025-02-11T20:57:24Z

Objective

As discussed in #17399, sending fake picking events (as you might want to do for clicking a focused element) is frustrating.

After discussing this with @aevyrie (notes here), part of the problem here is that our PointerId type is questionably designed. There are several small problems with it:

Constructing Uuids is very annoying.
Storing the id inside of each variant, rather than splitting the ID apart from the variants encourages inconsistent design and makes matching harder.
The Custom variant has unclear semantics. When mocking inputs for tests, you almost always want to pretend to be a real mouse or touch (often at a level above this, sending raw input events). But when sending picking events for "click on the focused element", it's not clear what whether you should pretend to be a mouse, a custom input, or even a touch.

Solution

Store the id of each pointer separately from the kind of pointer it is.
Standardize on a u64 identifier, and cut uuid from bevy_picking completely.
Add a PointerKind::Virtual variant with clear semantics for this sort of software-driven pointer that doesn't correspond to real hardware.

Future work

It would be nice to support multiple mice at some point in the future (I've had creative coding users asking me for this). Unfortunately, while this PR ensures that bevy_picking could easily accommodate it, winit still assumes a single mouse (and keyboard).

Unsurprisingly, patching winit is outside of the scope of this PR.

Testing

Existing tests pass. The picking examples seem to work fine, although I haven't tested them with touch inputs.

Migration Guide

PointerId has been refactored for consistency. Check the kind field to see the PointerKind of the pointer, and read the id field to recover the unique identifier.

To construct a PointerId::Mouse, use the PointerId::MOUSE constant. If you were using PointerId::Custom, either use PointerKind::Mouse (if you were mocking inputs for tests) or use PointerKind::Virtual if you were emulating a gamepad-controlled pointer or mocking pointer events to interact with various objects.

alice-i-cecile · 2025-02-11T20:59:11Z

crates/bevy_picking/src/pointer.rs

+        id: 0,
+        kind: PointerKind::Mouse,
+    };
+
    /// Returns true if the pointer is a touch input.
    pub fn is_touch(&self) -> bool {


I've left these utility methods alone to ease migration, but I'm not convinced they're particularly valuable now.

alice-i-cecile · 2025-02-11T20:59:27Z

examples/ui/directional_navigation.rs

-                    pointer_id: PointerId::Mouse,
+                    pointer_id: PointerId {
+                        id: 0,
+                        kind: PointerKind::Virtual,


No more pretending to be a mouse!

cart · 2025-02-12T22:13:45Z

crates/bevy_picking/src/pointer.rs

-    /// controlled cursor.
-    #[reflect(ignore)]
-    Custom(Uuid),
+pub struct PointerId {


Given that PointerId is now a common "thing" maybe it should just be PointerId(u64) and PointerKind would just be stored adjacent to it in places where that is relevant (ex: as a separate Component)? To distinguish between two pointers, don't we only need to compare / hash the ID, not the kind?

If it is not a common thing, then I think we should move back to a typed-wrapper to protect against comparing "unlike" ids across types.

Right now (and before this PR), the IDs are only meaningfully used by touches. In theory you could encode information for Virtual / Custom types in the u64 / UUID, but I'm not sure what you would use that for.

In either case, in order for two PointerId to match, both the kind and the number associated with it must match. As a result, I don't think we should split these up, but I'm fine to go back to an enum design here. Which do you prefer?

The current design, with split Id + kind.

enum PointerId, only Touch gets a u64.

enum PointerId, each kind has a u64.

I think I prefer solution 2, but went with solution 1 to ease migration.

I also prefer solution (2). I think thats the move.

ickshonpe · 2025-02-12T23:50:05Z

crates/bevy_picking/src/pointer.rs

+    /// A pointer that is controlled by a mouse.
+    ///
+    /// Not a rodent; the device that you move around on a desk.
+    Mouse,


I think that PointerKind is meant to describe the behaviour of the pointer, not the actual input device, but it seems a bit unclear. It would be nice to be able to support things like a "mouse" pointer that's controlled by a joystick or a mouse controlled first person pov with diegetic touch screen UI.

Yeah, the intent is that things like UI libraries will match on this variant, and behave accordingly.

NthTensor · 2025-02-13T00:30:01Z

I don't currently have time for review, but I'd like us to consider that winit is shipping a pointer abstraction in it's next major release that does away with the distinction between mouse and touch entirely.

alice-i-cecile · 2025-02-13T06:03:15Z

We could just do a single u64 then 🤔 I'd prefer to reduce controversy and get this merged though; there's relatively important work (like removing Interaction) that this is soft-blocking.

ickshonpe · 2025-02-13T10:05:31Z

crates/bevy_picking/src/pointer.rs

+#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash, Reflect)]
+#[reflect(Default, Debug, Hash, PartialEq)]
+pub enum PointerKind {
+    /// A pointer that is controlled by a mouse.


Suggested change

/// A pointer that is controlled by a mouse.

/// A pointer that behaves like a traditional mouse cursor.

ickshonpe · 2025-02-13T10:06:41Z

crates/bevy_picking/src/pointer.rs

+    ///
+    /// Not a rodent; the device that you move around on a desk.
+    Mouse,
+    /// A pointer that is controlled by a touch input.


Suggested change

/// A pointer that is controlled by a touch input.

/// A pointer that behaves like it is controlled by touch.

Like an obvious thing I think we'd want to support here is touch input emulation with the mouse?

ickshonpe · 2025-02-13T10:44:29Z

crates/bevy_picking/src/pointer.rs

+    /// A stable identifier for a specific pointer.
+    ///
+    /// This is typically provided by winit,
+    /// and is unique for each touch input or mouse.


This feels misleading. The id here corresponds to a specific touch on the device and it's only guaranteed to be unique for the duration of that touch I think.

ickshonpe · 2025-02-13T11:22:28Z

We could just do a single u64 then 🤔 I'd prefer to reduce controversy and get this merged though; there's relatively important work (like removing Interaction) that this is soft-blocking.

Maybe we shouldn't have the id here at all? The pointer entity which is spawned when the touch starts and despawned after it ends can be used to identify the touch within bevy. Then we just need to add a winit touch id -> pointer entity map somewhere that we can look up when we dispatch touch events.

edit: Implemented in PR #17847

colepoirier

The code looks correct to me, but given the comments of Cart in this PR and Aevyrie in ickshonpe's pr, I don't feel I have the requisite domain knowledge to properly weigh in on this.

alice-i-cecile added 2 commits February 11, 2025 12:40

Refactor PointerId to store the id and Kind in distinct fields

8841236

Use the Virtual variant for mocked directional navigation

921fd54

alice-i-cecile requested review from aevyrie, NthTensor and colepoirier February 11, 2025 20:57

alice-i-cecile commented Feb 11, 2025

View reviewed changes

alice-i-cecile requested a review from ickshonpe February 12, 2025 18:42

cart reviewed Feb 12, 2025

View reviewed changes

ickshonpe reviewed Feb 12, 2025

View reviewed changes

ickshonpe reviewed Feb 13, 2025

View reviewed changes

ickshonpe mentioned this pull request Feb 13, 2025

Remove the id from PointerId::Touch #17847

Open

colepoirier reviewed Feb 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split PointerId into distinct id and PointerKind fields #17808

Split PointerId into distinct id and PointerKind fields #17808

alice-i-cecile commented Feb 11, 2025

alice-i-cecile Feb 11, 2025

alice-i-cecile Feb 11, 2025

cart Feb 12, 2025

cart Feb 12, 2025 •

edited

Loading

alice-i-cecile Feb 12, 2025

cart Feb 13, 2025

ickshonpe Feb 12, 2025

alice-i-cecile Feb 12, 2025

NthTensor commented Feb 13, 2025

alice-i-cecile commented Feb 13, 2025

ickshonpe Feb 13, 2025

ickshonpe Feb 13, 2025

ickshonpe Feb 13, 2025 •

edited

Loading

ickshonpe commented Feb 13, 2025 •

edited

Loading

colepoirier left a comment

	/// A pointer that is controlled by a mouse.
	/// A pointer that behaves like a traditional mouse cursor.

	/// A pointer that is controlled by a touch input.
	/// A pointer that behaves like it is controlled by touch.

Split PointerId into distinct id and PointerKind fields #17808

Are you sure you want to change the base?

Split PointerId into distinct id and PointerKind fields #17808

Conversation

alice-i-cecile commented Feb 11, 2025

Objective

Solution

Future work

Testing

Migration Guide

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cart Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NthTensor commented Feb 13, 2025

alice-i-cecile commented Feb 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ickshonpe Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

ickshonpe commented Feb 13, 2025 • edited Loading

colepoirier left a comment

Choose a reason for hiding this comment

cart Feb 12, 2025 •

edited

Loading

ickshonpe Feb 13, 2025 •

edited

Loading

ickshonpe commented Feb 13, 2025 •

edited

Loading