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

[Merged by Bors] - Cleanup dynamic scene before building #6254

Closed

Conversation

mockersf
Copy link
Member

Objective

  • Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
  • Those entities are not really useful in the final DynamicScene

Solution

  • Add a method remove_empty_entities that will remove empty entities. It's not called by default when calling build, I'm not sure if that's a good idea or not.

@mockersf mockersf added the A-Scenes Serialized ECS data stored on the disk label Oct 14, 2022
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change and removed C-Bug An unexpected or incorrect behavior labels Oct 15, 2022
@alice-i-cecile
Copy link
Member

I'm on board. Yeah, unsure about making this process automatic. I gave you some docs to add at least. Might be worth calling out in the scene extraction methods?

@mockersf mockersf force-pushed the cleanup-dynamic-scene-before-building branch from 35fb966 to 9fd7db7 Compare December 17, 2022 08:18
Copy link
Contributor

@ramirezmike ramirezmike left a comment

Choose a reason for hiding this comment

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

looks good to me 👍

@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Dec 18, 2022
@james7132 james7132 added this to the 0.10 milestone Dec 19, 2022
@mockersf
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Dec 20, 2022
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
@bors
Copy link
Contributor

bors bot commented Dec 20, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Dec 20, 2022
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
@bors
Copy link
Contributor

bors bot commented Dec 20, 2022

Build failed:

@james7132
Copy link
Member

bors retry

bors bot pushed a commit that referenced this pull request Dec 20, 2022
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
@bors bors bot changed the title Cleanup dynamic scene before building [Merged by Bors] - Cleanup dynamic scene before building Dec 20, 2022
@bors bors bot closed this Dec 20, 2022
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
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-Code-Quality A section of code that is hard to understand or change 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.

4 participants