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

SubApp::extract cannot be called due to borrow checker #14003

Closed
anlumo opened this issue Jun 24, 2024 · 5 comments · Fixed by #14009
Closed

SubApp::extract cannot be called due to borrow checker #14003

anlumo opened this issue Jun 24, 2024 · 5 comments · Fixed by #14009
Labels
A-App Bevy apps and plugins C-Bug An unexpected or incorrect behavior

Comments

@anlumo
Copy link
Contributor

anlumo commented Jun 24, 2024

Bevy version

0.14.0-rc.3

What you did

I'm trying to call extract/update on a single SubApp instead of all at once.

What went wrong

Here is the code:

fn run_sub_app(app: &mut App, label: impl AppLabel) {
    let world = app.world_mut();
    if let Some(sub_app) = app.get_sub_app_mut(label) {
        sub_app.extract(world);
        sub_app.update();
    }
}
  • what were you expecting?

I'd like to run extract.

  • what actually happened?

Compiler error in the if let line: cannot borrow app mutable more than once, first borrow happens in the line defining the world variable.

I don't see a way to use the extract function at all, since the world I have to pass is always part of the enclosing context, and thus the borrow checker will never allow that.

Additional information

SubApps::update works, because it can do a partial borrow of self, but since I don't have access to SubApps, I can't do the same from outside the bevy_app crate.

This is an inherent limitation of Rust's borrow checker, because it can't do partial borrows on references returned from function calls.

@anlumo anlumo added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 24, 2024
@alice-i-cecile alice-i-cecile added A-App Bevy apps and plugins and removed S-Needs-Triage This issue needs to be labelled labels Jun 24, 2024
@alice-i-cecile
Copy link
Member

Can you give a bit more context on why you're using this pattern? I'd like to remove sub-apps completely, and want a better sense of the advanced use cases.

@anlumo
Copy link
Contributor Author

anlumo commented Jun 24, 2024

I have a UI system (not bevy's!) and want to display bevy views as UI views, and there might be multiples of them. There's a place in the app where I have to tell bevy to render that specific view to wgpu's queue right now (synchronously), which should be achievable by running sub_app.update();. This shouldn't render the other views, only the one I want right now (because there might be UI elements between two views).

It would be nice to have separate worlds for these views. I thought about using completely separate bevy Apps for this, but then I couldn't share entities when necessary. Also, I'd have to set up all systems for every App again and again.

@alice-i-cecile
Copy link
Member

Okay, so pretty standard multi-world / entity partitioning stuff. Thanks!

As for this issue, please feel free to make a simple PR to expose an API to do so.

@Soulghost
Copy link
Contributor

I have made a simple PR to expose an API called update_sub_app_by_label to resolve this issue, could you please verify if this is in line with our expectations?

@anlumo
Copy link
Contributor Author

anlumo commented Jun 25, 2024

I have made a simple PR to expose an API called update_sub_app_by_label to resolve this issue, could you please verify if this is in line with our expectations?

Yes, thank you!

github-merge-queue bot pushed a commit that referenced this issue Jun 25, 2024
…pp. (#14009)

# Objective

- Fixes #14003

## Solution

- Expose an API to perform updates for a specific sub-app, so we can
avoid mutable borrow the app twice.

## Testing

- I have tested the API by modifying the code in the `many_lights`
example with the following changes:
```rust
impl Plugin for LogVisibleLights {
    fn build(&self, app: &mut App) {
        let Some(render_app) = app.get_sub_app_mut(RenderApp) else {
            return;
        };

        render_app.add_systems(Render, print_visible_light_count.in_set(RenderSet::Prepare));
    }

    fn finish(&self, app: &mut App) {
        app.update_sub_app_by_label(RenderApp);
    }
}
```

---

## Changelog
- add the `update_sub_app_by_label` API to `App` and `SubApps`.

---------

Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
mockersf pushed a commit that referenced this issue Jun 25, 2024
…pp. (#14009)

# Objective

- Fixes #14003

## Solution

- Expose an API to perform updates for a specific sub-app, so we can
avoid mutable borrow the app twice.

## Testing

- I have tested the API by modifying the code in the `many_lights`
example with the following changes:
```rust
impl Plugin for LogVisibleLights {
    fn build(&self, app: &mut App) {
        let Some(render_app) = app.get_sub_app_mut(RenderApp) else {
            return;
        };

        render_app.add_systems(Render, print_visible_light_count.in_set(RenderSet::Prepare));
    }

    fn finish(&self, app: &mut App) {
        app.update_sub_app_by_label(RenderApp);
    }
}
```

---

## Changelog
- add the `update_sub_app_by_label` API to `App` and `SubApps`.

---------

Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-App Bevy apps and plugins C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants