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

Upstream and use bevy_mod_picking #12365

Closed
1 of 16 tasks
alice-i-cecile opened this issue Mar 7, 2024 · 4 comments
Closed
1 of 16 tasks

Upstream and use bevy_mod_picking #12365

alice-i-cecile opened this issue Mar 7, 2024 · 4 comments
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Editor Graphical tools to make Bevy games A-Input Player input via keyboard, mouse, gamepad, and more A-Picking Pointing at and selecting objects of all sorts A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible C-Tracking-Issue An issue that collects information about a broad development initiative D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 7, 2024

Why we need picking (and raycasting)

To be glib, bevy_mod_picking by @aevyrie allows users to point at things and click on them.
Picking is the pattern of turning pointer events (like mouse clicks or touches) into the hover/selection/deselection of objects (meshes, sprites, UI...) via hit testing, of which raycasting is one common form.

Bevy needs an equivalent set of tools for:

  1. General gameplay usage: raycasting is incredibly useful for everything from physics to line-of-sight to selecting units
  2. To replace our bespoke and inconsistent UI Interaction component and focus handling
  3. As an essential part of any balanced breakfast scene editor or interactive debugger

While the first application can and currently is solved by relying on an external crate, this is still limiting because:

  1. It puts pressure on the maintainer to continue to promptly update a vital dependency.
  2. It allows us to accidentally break a vital dependency on main, as it's not part of our CI.
  3. It worsens the new user experience, as they have to find and figure out external dependencies.
  4. It prevents us from building more realistic examples.

For the remaining use cases, we simply can't rely on an external crate that relies on Bevy due to circular dependencies which present severe technical and organizational challenges.

Why we should upstream rather than re-implement

Of course, the next question is "why should we upstream a crate, rather than simply building it ourselves!".
After all, it wasn't invented here, for a very narrow definition of "here".

However, as I previously discussed in bevyengine/rfcs#63, reinventing rather than upstreaming popular, functioning crates is both risky and wasteful.
While this functionality may seem simple at first glance, hard-won experience shows that there are a large number of ergonomic, design and performance concerns that must be carefully navigated.

bevy_mod_picking is a stable, beloved part of Bevy's ecosystem. While it may not be perfect in its current form, it decidedly works: with good performance, a lovely API and a generally low-bug experience. Building our own equivalent will take longer, involve dramatically more controversy, and leave us at a worse starting point.

Organizational strategy for upstreaming

I've been working closely with @aevyrie and other members of the community to plan this migration.
As part of that, we have a pair of junior volunteers from Purdue: Avi and Xavier who are helping us with this work (this issue is a sibling to #12349). Please be patient and kind with them as they work, but remember that they're here to learn and improve.

During this discussion, we've outlined a few key strategies to minimize the pain of this complex migration:

  1. Identify blockers and perform vital improvements in the upstream crate(s) before starting the migration PR. Once the entire crate(s) are updated, we can begin upstreaming.
  2. Split up the code into sizable and useful components, and upstream those in chunks. These PRs should be merged with minimal changes, to avoid stalling in review and accidentally introducing incompatibilities between our components.
  3. Perform code quality and documentation work on individual components in small PRs once they're merged. Do not make breaking changes here until the entire system is migrated, tested and integrated.
  4. Integrate the various components into the engine as they added, one area at a time, removing existing redundant solutions if we can do so without making breaking changes to the fragile new code.
  5. Once everything is integrated, we can take a look at how it behaves in context, and can then discuss more serious architectural refactors and small breaking changes.

The essential problems to avoid are:

  1. Complex PRs getting stuck in review and accumulating merge conflicts.
  2. Making a release of Bevy where the old solution is removed and the new solution is not yet ready.
  3. Making breaking changes part way through the migration, breaking the integration of other components.

If you have breaking concerns or ideas, please, please open issues to record them (and link this issue). We want to hear them, and to improve all areas of the engine: we just don't want the migration to fall into disarray :)

Technical pathway to upstreaming

Note: this has been updated to reflect evolving plans!

Related work

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Input Player input via keyboard, mouse, gamepad, and more A-UI Graphical user interfaces, styles, layouts, and widgets A-Editor Graphical tools to make Bevy games D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR labels Mar 7, 2024
@aevyrie
Copy link
Member

aevyrie commented Mar 7, 2024

Thank you Alice, reading through this now. 🙂

Small correction of a common misconception. Picking does not require raycasting, that is just one of the hit testing methods.

@brandon-reinhart
Copy link
Contributor

I use both crates extensively and have written extensions in my own game. I will keep an eye on this and will be available for code reviews. Excited to see this start!

@matiqo15 matiqo15 added the A-Dev-Tools Tools used to debug Bevy applications. label Apr 3, 2024
@alice-i-cecile alice-i-cecile moved this to Active: bevy_mod_picking upstreaming in Alice's Work Planning May 1, 2024
@alice-i-cecile alice-i-cecile added X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers and removed X-Controversial There is active debate or serious implications around merging this PR labels May 2, 2024
@alice-i-cecile alice-i-cecile added the A-Picking Pointing at and selecting objects of all sorts label Jun 5, 2024
@alice-i-cecile alice-i-cecile moved this to Active Working Groups in Editor Aug 19, 2024
@alice-i-cecile alice-i-cecile added the C-Tracking-Issue An issue that collects information about a broad development initiative label Aug 19, 2024
@NthTensor
Copy link
Contributor

NthTensor commented Aug 20, 2024

I'd like to give a quick update here laying out the next steps. Most of the existing mod_picking code has been moved over, and but the work is still ongoing. Picking had basically has two flaws

  • You can't register multiple handles.
  • Pointer events are not ordered properly.

We managed to (mostly) resolve the first one of these issues with observers. Here's how I want to resolve the second:

  1. Promote bevy_winit::WinitEvent to bevy_window::WindowEvent and use it as the basic input stream for picking.
  2. Unify InputMove and InputPress into PointerInput, and update the dispatchers to preserve order.
  3. Unify the multiple systems that turn the hover map and pointer input unto Pointer<X> events. This will be a single system that triggers observers in a way that matches the input stream ordering.

These are implemented here: #14862.

This will conclude the minimum necessary work for the up-streaming (for the core logic at least, there may still be work in documenting, testing, and adding back-ends). Here's a few other things we will want to do after this is complete:

  • Move most of the pointer abstraction from picking into input. Winit is also moving to a unified pointer abstraction, so we will need to do this anyway at some point.
  • Allow pointers to have a sequence of locations in a single frame, capturing sub-frame motion and interaction, and making interaction event dispatch much more accurate and reliable.

github-merge-queue bot pushed a commit that referenced this issue Aug 26, 2024
# Objective

Add `bevy_picking` sprite backend as part of the `bevy_mod_picking`
upstreamening (#12365).

## Solution

More or less a copy/paste from `bevy_mod_picking`, with the changes
[here](aevyrie/bevy_mod_picking#354). I'm
putting that link here since those changes haven't yet made it through
review, so should probably be reviewed on their own.

## Testing

I couldn't find any sprite-backend-specific tests in `bevy_mod_picking`
and unfortunately I'm not familiar enough with Bevy's testing patterns
to write tests for code that relies on windowing and input. I'm willing
to break the pointer hit system into testable blocks and add some more
modular tests if that's deemed important enough to block, otherwise I
can open an issue for adding tests as follow-up.

## Follow-up work

- More docs/tests
- Ignore pick events on transparent sprite pixels with potential opt-out

---------

Co-authored-by: Aevyrie <[email protected]>
@alice-i-cecile
Copy link
Member Author

Closing as "effectively complete".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Editor Graphical tools to make Bevy games A-Input Player input via keyboard, mouse, gamepad, and more A-Picking Pointing at and selecting objects of all sorts A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible C-Tracking-Issue An issue that collects information about a broad development initiative D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
Status: Active: bevy_mod_picking upstreaming
Status: Active Working Groups
Development

No branches or pull requests

5 participants