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

Remove Handle<T> trait implementations that are dependent on Component #15749

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Oct 8, 2024

Objective

Solution

  • Remove unused ExtractComponent trait implementation for Handle<T>
  • Remove unused ExtractInstance trait implementation for AssetId
    • Although the ExtractInstance trait wasn't used, the AssetIds were being stored inside of ExtractedInstances which has an ExtractInstance trait bound on its contents.
      I've upgraded the RenderMaterialInstances type alias to be its own resource, identical to ExtractedInstances<AssetId<M>> to get around that with minimal breakage.

Testing

Tested many_cubes, rendering did not explode

@tim-blackbird tim-blackbird added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 8, 2024
@tim-blackbird tim-blackbird added this to the 0.15 milestone Oct 8, 2024
pub type RenderMaterialInstances<M> = ExtractedInstances<AssetId<M>>;
/// Stores all extracted instances of a [`Material`] in the render world.
#[derive(Resource, Deref, DerefMut)]
pub struct RenderMaterialInstances<M: Material>(pub EntityHashMap<AssetId<M>>);
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I missing something? This doesn't seem to be used anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You must be, it's being use in quite a few places :)

None of them show up in the diff because it's API is exactly what it was, in case that's where you were looking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants