Skip to content

Conversation

Glory2Antares
Copy link

@Glory2Antares Glory2Antares commented Aug 31, 2025

Objective

  • Since Vec<Handle<A>> and Vec<UntypedHandle> implement VisitAssetDependencies users might expect [Handle<A>; N] and [UntypedHandle; N] to also implement VisitAssetDependencies, but this is currently not the case.
  • The #[dependency] attribute from the Asset derive macro should work with an implementation.

Solution

Implement VisitAssetDependencies for [Handle<A>; N] and [UntypedHandle; N]. The implementations are based on the corresponding ones for Vec.

Testing

A test for compatibility with the derive macros was added for [Handle<A>; N] and [UntypedHandle; N].


Showcase

Before:

// Can't use the Asset macro with a custom VisitAssetDependencies impl 
#[derive(TypePath)]
struct MyAsset {
    images: [Handle<Image>; 5],
}

impl VisitAssetDependencies for MyAsset {
    fn visit_dependencies(&self, visit: &mut impl FnMut(UntypedAssetId)) {
        for image in &self.images {
            visit(image.id().untyped());
        }
    }
}

impl Asset for MyAsset {}

After:

#[derive(Asset, TypePath)]
struct MyAsset {
    #[dependency]
    images: [Handle<Image>; 5],
}

@Glory2Antares Glory2Antares marked this pull request as ready for review August 31, 2025 00:23
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon labels Sep 1, 2025
Copy link
Contributor

@andriyDev andriyDev left a comment

Choose a reason for hiding this comment

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

Nice! Simple and straight forward!

@andriyDev andriyDev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 2, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants