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

Manual "Reflect Value" AssetPath impl to fix dynamic linking #9752

Merged
merged 5 commits into from Sep 13, 2023

Conversation

cart
Copy link
Member

@cart cart commented Sep 10, 2023

Objective

Fix #9747

Solution

Linkers don't like what we're doing with CowArc (I'm guessing it has something to do with ?Sized). Weirdly the Reflect derive on AssetPath doesn't fail, despite CowArc not implementing Reflect.

To resolve this, we manually implement "reflect value" for AssetPath<'static>. It sadly cannot use impl_reflect_value because that macro doesn't support static lifetimes.

@cart cart added C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds labels Sep 10, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Sep 10, 2023
Co-authored-by: Martin Dickopp <martin@zero-based.org>
Copy link
Contributor

@mdickopp mdickopp left a comment

Choose a reason for hiding this comment

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

Fixes the issue for me.

@alice-i-cecile alice-i-cecile 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 Sep 13, 2023
@cart cart added this pull request to the merge queue Sep 13, 2023
Merged via the queue into bevyengine:main with commit 5fb3eb5 Sep 13, 2023
22 checks passed
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
…ine#9752)

# Objective

Fix bevyengine#9747

## Solution

Linkers don't like what we're doing with CowArc (I'm guessing it has
something to do with `?Sized`). Weirdly the `Reflect` derive on
`AssetPath` doesn't fail, despite `CowArc` not implementing `Reflect`.

To resolve this, we manually implement "reflect value" for
`AssetPath<'static>`. It sadly cannot use `impl_reflect_value` because
that macro doesn't support static lifetimes.

---------

Co-authored-by: Martin Dickopp <martin@zero-based.org>
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 C-Bug An unexpected or incorrect behavior 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.

Build failure when dynamic_linking feature is used
3 participants