Skip to content

Conversation

@greeble-dev
Copy link
Contributor

@greeble-dev greeble-dev commented Jan 2, 2026

Objective

Fix a panic in mesh picking if a mesh is RenderAssetUsages::RENDER_WORLD only. The panic was reported in #22206, although that issue is framed as a broader concern and so is not resolved by this PR. The problem was introduced in #21732.

Solution

Changed mesh accesses to use the non-panicking try_ variants. This means extracted meshes are silently ignored as before.

I also did a quick scan to see if could spot any other engine crates that might have the same issue - didn't see any but could easily have missed one.

Testing

Tested by modifying the mesh_picking example:

+    let mut p = Mesh::from(Cuboid::default());
+    p.asset_usage = bevy_asset::RenderAssetUsages::RENDER_WORLD;
     let shapes = [
-        meshes.add(Cuboid::default()),
+        meshes.add(p),

@greeble-dev greeble-dev added C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Picking Pointing at and selecting objects of all sorts labels Jan 2, 2026
@greeble-dev greeble-dev added this to the 0.18 milestone Jan 2, 2026
@greeble-dev
Copy link
Contributor Author

Adding to the 0.18 milestone - the reporting issue is tagged as 0.18, and the bug it fixes is a panic introduced in 0.18.

Copy link
Contributor

@kfc35 kfc35 left a comment

Choose a reason for hiding this comment

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

LGTM for correctness, checked out this branch and verified that the example does not crash with this code

@robtfm
Copy link
Contributor

robtfm commented Jan 3, 2026

code is fine.

to consider it properly closed i think we should warn_once! when the mesh data is extracted - something like

let positions = mesh
        .try_attribute_option(Mesh::ATTRIBUTE_POSITION) // only errors if extracted
        .inspect_err(|_| warn_once!("must use main world"))
        .ok()?? // else returns Ok(Option<Values>) so we must ?? to get the data out
        .as_float3()?;

or

let positions = match mesh.try_attribute(Mesh::ATTRIBUTE_POSITION) {
  Err(Extracted) => { warn_once!("extracted"); return None }
  Err(Missing) => { warn_once!("missing"); return None }
  Ok(data) => data.as_float3()?;
}

@JMS55 JMS55 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 3, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 5, 2026
Merged via the queue into bevyengine:main with commit 2ccdd50 Jan 5, 2026
51 checks passed
cart pushed a commit that referenced this pull request Jan 8, 2026
## Objective

Fix a panic in mesh picking if a mesh is
`RenderAssetUsages::RENDER_WORLD` only. The panic was reported in
#22206, although that issue is
framed as a broader concern and so is not resolved by this PR. The
problem was introduced in #21732.

## Solution

Changed mesh accesses to use the non-panicking `try_` variants. This
means extracted meshes are silently ignored as before.

I also did a quick scan to see if could spot any other engine crates
that might have the same issue - didn't see any but could easily have
missed one.

## Testing

Tested by modifying the `mesh_picking` example:

```diff
+    let mut p = Mesh::from(Cuboid::default());
+    p.asset_usage = bevy_asset::RenderAssetUsages::RENDER_WORLD;
     let shapes = [
-        meshes.add(Cuboid::default()),
+        meshes.add(p),
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Picking Pointing at and selecting objects of all sorts C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants