Changed the way cursors are defined in bevy_feathers.

[viridia]

Originally, we re-used the CursorIcon component which was intended to be placed on the window, and expanded its use so that it could be placed on any hoverable entity.

In this PR, we restore CursorIcon to its original usage, and instead introduce a new type EntityCursor intended to be placed on hoverable entities.

Part of the motivation for this is that it will make it easier to support custom cursors in BSN templates without having to add new trait impls in bevy_winit.

Bikeshedding on the name EntityCursor is welcome. Originally I had thought NodeCursor, but it's not just for Nodes, it can be placed on any hoverable entity.

Note: this was discussed in Discord with @cart

[mgi388]

Out of interest, does it make sense to make the default cursor a component on window rather than a global resource? Presumably you could then define the per-window default as well.

Is there a case where there’s something further up the tree than a window which would make this suggestion fall apart?

[viridia]

I prefer not to add features based on hypothetical use cases - ones that may or may not ever come up. The vast majority of apps only have one window.

[alice-i-cecile]

I don't see why you would ever want to have a distinct default cursor per window.

[mgi388]

The reason I mentioned this is that it seemed like a natural place to put it instead of it being a resource. That is to say, if you only have 1 window most of the time, then your window is your “resource singleton” anyway. The cursor already “belongs to” the window (via CursorIcon) and the code already iterates over all windows AFAICS and so it just felt natural.

But yeah I’m not at all fussed, was just a thought.

[mgi388]

Feel free to resolve this, I can’t.

[mogambro]

For what it's worth, @mgi388 that's actually what we do in one app for work (although Python and GTK) - the different windows have different usages and different cursors for "prod" or "test" environment.

(Why that's in one app and not two: don't ask!)

[mgi388]

Yeah I could see use cases exist here. Like maybe you have a map in a separate window that you default to a grab cursor (because you just pan the map around using your cursor) and then the primary window is just a pointer.

I guess my point really was that if it was a component per window entity then a) that use case would magically already be solved and b) it seems like a concept that belongs to the window entity so may as well add it there rather than as a global resource, which felt cleaner to me (e.g. you wouldn’t have a dangling resource that is only relevant to if you have a window in your app).

But yeah, I’m still not fussed, I’m sure it can be changed in the future if needed.

[alice-i-cecile]

Small naming + docs suggestion, then LGTM.

[viridia]

@alice-i-cecile Ideally this whole thing should move to bevy_picking; the only possible reason why you might not is that this would add a dependency bevy_picking -> bevy_winit, but since it already depends on bevy_window I don't see why this would be a problem.

[viridia]

Also, I don't know why the check-advisories CI is whinging.

[cart]

but since it already depends on bevy_window I don't see why this would be a problem.

The issue here is that bevy_winit is considered a "backend" for bevy_window, which is the backend-agnostic interface. Ideally bevy_picking is "backend independent". Aka it doesn't care if someone wants to use bevy_sdl2 instead of bevy_winit.

[viridia]

but since it already depends on bevy_window I don't see why this would be a problem.

The issue here is that bevy_winit is considered a "backend" for bevy_window, which is the backend-agnostic interface. Ideally bevy_picking is "backend independent". Aka it doesn't care if someone wants to use bevy_sdl2 instead of bevy_winit.

If that's the case, then maybe we need to abstract out cursor definitions from bevy_winit.

[cart]

If that's the case, then maybe we need to abstract out cursor definitions from bevy_winit.

Agreed. I think this was attempted in the past (I vaguely remember advocating for it), but I also vaguely remember there being something that made it non-trivial.

[alice-i-cecile]

If that's the case, then maybe we need to abstract out cursor definitions from bevy_winit.

Agreed. I think this was attempted in the past (I vaguely remember advocating for it), but I also vaguely remember there being something that made it non-trivial.

IIRC the problem was horrible circular dependencies with respect to rendering, assets, windowing and bevy_image. Same flavor of problem as our yet-unfixed "set window decoration" stuff.

[viridia]

Added #20182

[viridia]

Fixes #20283

Also, I pushed a new version which streamlines the logic for setting the window cursor considerably.

[alice-i-cecile]

@viridia can you tackle merge conflicts for this now? I think this should be the last round of them.

[viridia]

@alice-i-cecile Should be good to go once the CI finishes.