Adding Cursor Events as Part of System about bevy_mod_picking HOT 14 CLOSED

This is finished! Creating Pull Request.

#48

My approach was to make it a plugin / component. You can then query the state directly on the entities returned in whatever Query you're using. This allows you to define behavior on the target Entity/Component Set directly which I think is a little more ergonomic. I don't think this will result in significant performance implications as its just another system that iterates over a Query<(&InteractableMesh, Entity)>

We could add a resource that gets the entity for mouse button / hover / enter / exit state but I feel like that may butt up a little more against Bevy's ECS design.

from bevy_mod_picking.

I think the ClickDown approach works for running logic on data captured in the closure / passed into it, but I have a feeling it won't be very useful in practice because you can't pass arbitrary ECS data into it.

CursorEvents would work for "generic" use cases, but as you mentioned it would have performance implications because you need to do a full scan through events you might not care about. This probably won't be a huge deal, but its still worth thinking about.

In general this is very similar to the "click event" problem we solved in Bevy UI. Currently we're using an Interaction component on each "interactable" ui element, and we set the state on it (hovered, clicked, none) whenever something changes.

The "change tracking" queries then allow us to write systems that react to changes in state:
https://github.com/bevyengine/bevy/blob/master/examples/ui/button.rs

I'm not fully convinced that is the right pattern for bevy_ui, but it certainly works and is nice to consume.

from bevy_mod_picking.

Some notes from reviewing the code for potential improvements:

• interactable::cursor_events() relies on the pick state being populated from the body of the plugin. Can we completely eliminate this resource, and store pick data directly in the picked entity's component? (I realize that wasn't an option for this initial PR)
• InteractableMesh seems a bit heavy to me. There should only be one source of truth for a PickIntersection, I think this should only be stored once. Additionally, I think enums can improve the expression of some of the exclusive states. I think the bools could all be replaced with a single enum which would encode the fact that the mouse can only be (hovered xor entered xor exited).
• I'm not convinced that including the mouse button press events is worth the tradeoffs - that seems like something better left to the user, and is very simple for them to do. If I want to handle events with multiple button presses or maybe in conjunction with a keyboard button, I can no longer use the plugin's implementation, and trying to handle all these cases in this plugin seems out of scope. However, handling mouseover events is not trivial, and is definitely in the scope of this plugin.

Here's an example of how I might use algebraic types to describe all possible states:

pub enum Topmost {
    True,
    False,
}

pub enum InteractionState {
    Hovering(Topmost, PickIntersection)
    JustEntered(Topmost, PickIntersection)
    JustExited
    NoInteraction
}

pub struct InteractableMesh {
    state: InteractionState
}

from bevy_mod_picking.

We could definitely refactor this to use the ray-casting stuff directly, I went ahead and used the PickState resource since that system is already running as part of the plugin. We may be duplicating effort unless we decouple stuff where the picking plugin isn't already running.

I thought about adding an enum, but decided against it since you can technically be both 'Hovering' and 'Just Entered'

If we make them exclusive, you would have to duplicate any logic for the Hover state to the Just Entered State, otherwise that 'hover' logic wouldn't run on the first frame that the cursor is over the mesh. If an enum is preferred I don't have a problem with that.

Per the buttons, InteractableMesh is initialized with a list of MouseButtons that the mesh 'listens' for. When the component is updated it populates a vec:(MouseButton, PickIntersection) with any of those buttons that are pressed, just_pressed, or just_released allowing you to handle multiple buttons. I can see the case for removing the mouse buttons from the plugin, but I thought the pattern of specifying what buttons the component listens to added some centralization that would make life easier.

Keyboard Buttons, were intentionally excluded, but the user's system can pull in the requisite resources in addition to the InteractableMesh component in their Query.

This was written with the goal of eventually subsuming the PickState resource as something that the User has to interact with directly. All of the examples can be refactored to work with this model which keeps all the information in one place.

Essentially I wanted to encapsulate everything needed for problems like "Do Something when the mouse first enters the mesh and releases the mouse button"

from bevy_mod_picking.

FYI, I'm in the process of taking @UnFlimFlammable's work, integrating this into the main picking logic, and removing any redundant code.

from bevy_mod_picking.

Howdy @UnFlimFlammable and @guimcaballero, PR #52 is ready to merge. However, I'd appreciate feedback from you first. This is the result of a few rounds of experimentation, seeing how bare minimum I could make events while still making them useful.

I ended up with hover events (JustEntered, JustExited) and mouse down events (MouseJusePressed, MouseJustReleased), as well as current hover state. This is all stored inside the InteractableMesh component, as suggested in the posts above. I'd suggest running the events example (formerly interactable_cube) to see what it looks like. Here is some example output:

ENTITY: 2v0, HOVER: true, HOVER EVENT: None, CLICK_EVENT: MouseJustReleased
ENTITY: 2v0, HOVER: true, HOVER EVENT: None, CLICK_EVENT: MouseJustPressed
ENTITY: 2v0, HOVER: true, HOVER EVENT: None, CLICK_EVENT: MouseJustReleased
ENTITY: 1v0, HOVER: true, HOVER EVENT: JustEntered, CLICK_EVENT: None
ENTITY: 2v0, HOVER: false, HOVER EVENT: JustExited, CLICK_EVENT: None
ENTITY: 1v0, HOVER: false, HOVER EVENT: JustExited, CLICK_EVENT: None

I've expanded support for pick groups through all the things (highlighting, selection, interaction). This is especially important for multi-window and PIP uses. In the process, I added a bunch of Default impls to cover the primary use case of never touching PickGroups.

More importantly, I've dogfooded this by rewriting the selection and highlighting logic using the new events system. It was super easy, pleasant to use, and made the code much more readable/auditable! The selection system is now ludicrously succinct:

if mouse_button_inputs.just_pressed(MouseButton::Left) {
    for (mut selectable, interactable) in &mut query.iter_mut() {
        selectable.selected = interactable.groups_just_pressed(MouseButton::Left);
    }
}

To keep the plugin lean, the InteractableMesh components and systems have been made into a plugin that is opt-in. Because the selection and highlighting systems rely on this, it is a prerequisite for those systems:

.add_plugin(InteractablePickingPlugin)

from bevy_mod_picking.

I'm going to merge this into the main branch to allow @nicbarker to merge doc changes. I'll leave this open for feedback as I prepare for a Bevy 0.3 compatible crates.io release. 😃

from bevy_mod_picking.

Per a Discord discussion with @cart I think we should do the following:

Implement our solution the same way Bevy UI handles click events with a InteractableMesh component that contains the current event state. We can add a system to the EventUpdate stage that will iterate over all entities under the cursor and update the InteractableMesh with the relevant data.

When the scheduler moves to the Update stage the updated data will be available for consumption by normal systems.

This would resolve the scoping issues of the closures as well as the performance issues of the system / resource approach.

from bevy_mod_picking.

Making the UI event handling more inline with Bevy sounds good to me! It's pretty gross (but functional) right now. 😄

Implement our solution the same way Bevy UI handles click events with a InteractableMesh component that contains the current event state.

That's kinda the direction I was heading, especially because it seems to be more Bevy-idiomatic. My stumbling block until now has been that I was unsure if it would have performance implications as the number of entities increase. For example, there is probably only going to be one entity that is hovered or selected in most cases. The simplest solution is to store a list of entities that are affected, which is then just a hashmap lookup away (O(1)). If we store selection state as a component on an entity, we now need to look at every entity to find the one(s) with the state we are interested in (O(n)). However, I assume change tracking gets around this problem by storing a short list of component state diffs (O(1)).

What do you see as the next steps? Is this something we could work on together?

from bevy_mod_picking.

@aevyrie yeah the "scanning" is a tradeoff, but i think even for large numbers of entities it wont be a huge deal due to the cache wins we get from this model. it would be interesting to experiment with an ecs scheduler that is something closer to "iterating change events and running relevant systems". this would be trivial / probably a clear win for a single "change event", but it would still require hashing, allocations, and/or scans for "queries with multiple change events", to the point that it could become more expensive than a single full scan.

Fortunately, thats all "under the hood" and if we find optimizations that are clearly better, we can apply them without api changes.

from bevy_mod_picking.

@cart That all sounds very reasonable to me. Thanks for chiming in over here!

from bevy_mod_picking.

Because @UnFlimFlammable did such a nice job of keeping this new code independent from everything else 😄, I'm going to merge this ASAP so we can play around with this new cursor event system. We can keep this thread open to discuss as we converge on a solution for handling cursor events.

from bevy_mod_picking.

I thought about adding an enum, but decided against it since you can technically be both 'Hovering' and 'Just Entered'
If we make them exclusive, you would have to duplicate any logic for the Hover state to the Just Entered State, otherwise that 'hover' logic wouldn't run on the first frame that the cursor is over the mesh. If an enum is preferred I don't have a problem with that.

That's a great point! This could be solved a few ways - preferably by reorganizing the enums - there is probably a better way to do it than my first attempt at an example. 😄

Ultimately, I prefer to use enums instead of bools in cases where implicit intent and constraints can be made explicit.

I can see the case for removing the mouse buttons from the plugin, but I thought the pattern of specifying what buttons the component listens to added some centralization that would make life easier.

My preference would be to avoid making the plugin opinionated about how to handle things like mouse/keyboard/gamepad/etc inputs, rather it would only tell you that some entity has been just_picked, just_exited, or hovered. Consider that this needs to be general for any input, not just mice, and I want to constrain scope (at least for now) to make what we have solid and generalizable.

Essentially I wanted to encapsulate everything needed for problems like "Do Something when the mouse first enters the mesh and releases the mouse button"

Understood! I think that providing state transition information (just_picked, etc.) is enough to cover 99% of the work without getting in the user's way. The last 1% is checking the other prerequisites are met (is button x + y down) which is simple to do with bevy.

from bevy_mod_picking.

Closing this with the release of 0.2. Thanks to everyone who contributed!

from bevy_mod_picking.