Skip to content

Don't use ScenePatchInstance for queued scenes#24474

Open
cart wants to merge 2 commits into
bevyengine:mainfrom
cart:bsn-queue-fix
Open

Don't use ScenePatchInstance for queued scenes#24474
cart wants to merge 2 commits into
bevyengine:mainfrom
cart:bsn-queue-fix

Conversation

@cart
Copy link
Copy Markdown
Member

@cart cart commented May 28, 2026

Objective

Fix #24320

Solution

Don't use ScenePatchInstance for queued scenes. Instead, use QueuedScenes directly.

Testing

The example in #24320 now works.

@cart cart added this to the 0.19 milestone May 28, 2026
@cart cart added C-Bug An unexpected or incorrect behavior A-Scenes Composing and serializing ECS objects labels May 28, 2026
Copy link
Copy Markdown
Member

@laundmo laundmo left a comment

Choose a reason for hiding this comment

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

Simple fix. at the cost of a resource access, but i think thats inevitable here and probably costs less than the archetype move of the insert.

@laundmo
Copy link
Copy Markdown
Member

laundmo commented May 29, 2026

Wait, is WaitingScenes the correct resource here? Theres also QueuedScenes which is what ScenePatchInstance was used for.

It seems like WaitingScenes will skip a lot of work which QueuedScenes does, and i'm not sure of the effects of skipping this work. Plus, the documentation of WaitingScenes is:

/// A [Resource] that tracks entities / scenes that are waiting for an asset to load which doesn't seem to fit.

@laundmo laundmo self-requested a review May 29, 2026 12:21
@cart
Copy link
Copy Markdown
Member Author

cart commented May 29, 2026

Wait, is WaitingScenes the correct resource here?

Yeah this is fine.

/// A [Resource] that tracks entities / scenes that are waiting for an asset to load which doesn't seem to fit.

Functionally, all "queued scenes" are "waiting for the scene to load". QueuedScenes largely just facilitates an initial check to see if the scene is loaded (and then it puts it in the "waiting queue" if it isn't). I'm thinking this initial check isn't actually necessary / theres probably some simplification potential here.

@cart
Copy link
Copy Markdown
Member Author

cart commented May 29, 2026

@laundmo sadly WaitingScenes can't replace QueuedScenes (and can't be what we use for queue_X actions) because its used in a resource_scope, which prevents nested queues. Thats why the test is failing. QueuedScene has a double buffered mem-swap, which solves the problem. I'm going to switch to using that.

);

loop {
core::mem::swap(&mut *world.resource_mut::<QueuedScenes>(), &mut queued);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved this loop until after the "asset load event" processing to ensure that we spawn queued scenes that are produced from the spawning of those scene asset loads now rather than waiting a frame.

I've also moved the mem::swap into the loop to ensure we check for newly queued spawns after spawning the current queue.

@kfc35 kfc35 added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Scenes Composing and serializing ECS objects C-Bug An unexpected or incorrect behavior S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScenePatchInstance fails silently when spawning via bsn!

4 participants