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

Add an example showing how to edit a scene before it's spawned #5872

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Sep 3, 2022

Objective

  • Show how to edit a scene before it's spawned
  • Small API improvements around Scene/DynamicScene

@mockersf mockersf added C-Examples An addition or correction to our examples A-Scenes Serialized ECS data stored on the disk labels Sep 3, 2022
@alice-i-cecile alice-i-cecile added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label Sep 3, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

A few comments. This also needs a Migration Guide as there are breaking changes :)

@@ -25,6 +26,18 @@ impl Scene {
Self { world }
}

/// Create a new scene from a given dynamic scene.
Copy link
Member

Choose a reason for hiding this comment

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

A little more explanation around the type registry parameter would be appreciated.

.run();
}

pub const CAKE_WITH_LIGHT_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Scene::TYPE_UUID, 1);
Copy link
Member

Choose a reason for hiding this comment

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

I would add a comment explaining how these constants were chosen.

Copy link
Member Author

Choose a reason for hiding this comment

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

would you believe me if I said "randomly selected value"? 😄

Copy link
Member

Choose a reason for hiding this comment

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

grin Of course ;) Good to explain to the users though.

.despawn_recursive();

// add the modified scenes to the assets
scenes.set_untracked(CAKE_WITH_LIGHT_HANDLE, scene_with_lights);
Copy link
Member

Choose a reason for hiding this comment

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

set_untracked feels like a very unintuitive name for this behavior. Should it be renamed?

Copy link
Member Author

Choose a reason for hiding this comment

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

set_untracked creates an asset that is not tracked for automatic cleanup, the name seems right?

Copy link
Member

Choose a reason for hiding this comment

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

Why not create_untracked then? set to me implies that it's setting an existing asset.

Or even create_durable or something, to better communicate the intent.

@mockersf mockersf marked this pull request as draft September 18, 2022 14:13
@mockersf
Copy link
Member Author

converting to draft as most comments are related to how to do asset post processing, and it's just not something that can be pretty in Bevy for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Scenes Serialized ECS data stored on the disk C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Examples An addition or correction to our examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants