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

bevy_picking release notes #1806

Merged
merged 13 commits into from
Nov 15, 2024

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Nov 14, 2024

I still need to add an image and a usage section to this, but the simple_picking example is currently broken so I can't steal its approach.

EDIT: seems fine, I just misread what it's supposed to do.

@alice-i-cecile alice-i-cecile marked this pull request as ready for review November 14, 2024 23:10
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well written! I have a suggestion around breaking up the example just to avoid having a single large block of code, but that's a stylistic choice and I understand wanting to keep it all together as a single "real" example, rather than just highlighting some snippets.

@@ -1,4 +1,117 @@
<!-- Add mesh picking backend and `MeshRayCast` system parameter -->
<!-- https://github.com/bevyengine/bevy/pull/15800 -->

<!-- TODO -->
![A collection of geometric shapes, with a pointer showing a point on a hovered mesh. The indicator is perpendicular to the surface.](mesh_picking.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame that mesh picking is called out further down as being disabled by default and discouraged due to performance concerns, but then the title image for this feature is explicitly mesh picking. I don't think there's anything that can be done about it though. These are real concerns, and mesh picking is the best visual representation of the feature.

@aevyrie
Copy link
Member

aevyrie commented Nov 15, 2024

To be honest, I think the fears about perf on raycast picking are overblown. The way picking is designed, switching from the mesh raycast backend, to rapier, and then to avian, has no impact on the picking code, it is completely agnostic to the hit test provider. This means there is really no risk in encouraging people to use the raycasting backend - it is a great way to get started with 3d without needing to worry about a bunch of setup, and if the thing you make turns out to outlive the toy stage, switching over to a physics engine has no real impact on your picking code.

Plus, let's be honest with ourselves, 99% of users have small scenes and low poly assets, and the raycast performance is never going to show up on a profile for these people. Considering I can use it on Caldera without issue, I am really not worried.

alice-i-cecile and others added 2 commits November 14, 2024 21:13
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
@alice-i-cecile
Copy link
Member Author

Caldera is pretty impressive! I've swapped to a gentler framing @aevyrie, and also adopted your suggested example. I personally really like the UI applications, but since there's a bit of a footgun there still we shouldn't demo it so prominently yet. Also: shorter is better!

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 15, 2024
Merged via the queue into bevyengine:main with commit 50f9709 Nov 15, 2024
10 checks passed
@alice-i-cecile alice-i-cecile deleted the bevy-picking-notes branch November 15, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants