Remove dummy() from WindowId and DeviceId and use Option<DeviceId>#3864
Remove dummy() from WindowId and DeviceId and use Option<DeviceId>#3864kchibisov merged 1 commit intorust-windowing:masterfrom
dummy() from WindowId and DeviceId and use Option<DeviceId>#3864Conversation
madsmtm
left a comment
There was a problem hiding this comment.
I think dummy() on WindowId was mostly useful back when we had Event::WindowEvent(WindowId, WindowEvent), and users wanted to create Event for testing.
|
|
||
| impl DeviceId { | ||
| #[cfg(test)] | ||
| pub const fn dummy() -> Self { |
There was a problem hiding this comment.
Still need to remove this and other fn dummy().
Or do we use it for testing anywhere that I don't know about?
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't see that much value in that test compared to the many extra functions that we need to make it work - but I don't have a strong preference either way here.
There was a problem hiding this comment.
I feel the same and would be in favor of removing it.
Unless I hear disagreement I'm gonna go ahead with that when the PR is ready.
3ac49d9 to
71faa98
Compare
`::dummy` was used for testing purposes and became redundant after adding e.g. `from_raw` and `into_raw` methods on `Id` types.
07f161b to
680c52a
Compare
kchibisov
left a comment
There was a problem hiding this comment.
Brought it up to date, should be good now.
This was discussed in the last meeting but it was so late only @kchibisov and myself were around so I'm gonna re-iterate what we discussed here.
WindowId,DeviceIdand the newly addedFingerId, have only one purpose: making sure that users are able to differentiate between events coming from different devices.dummy()however entirely circumvents this property, because it is always equal itself. Ideallydummy()would never be equal itself, but that's not possible while we want to implementHash(unless we add a counter ...).This is in a similar vein to #3795.
However, this assumption was made under the fact hat adding
Option<DeviceId>was not necessary because only Web is usingdummy()internally. Turned out this isn't exactly true: every backend has at least some events where it is unable to emit a uniqueDeviceIdfor a device. Some backends, like Wayland and Orbital, don't use anyDeviceIdat all, making all devices appear equal!So with that in mind, I also replaced all
DeviceIds withOption<DeviceId>.Surprisingly enough this wasn't necessary for
FingerIdbecause every backend that implements touch emits a proper finger ID.Based on #3795.