Skip to content

Meta-issue for improving reflect-deserialized asset loading #15518

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

Open
aecsocket opened this issue Sep 29, 2024 · 0 comments
Open

Meta-issue for improving reflect-deserialized asset loading #15518

aecsocket opened this issue Sep 29, 2024 · 0 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Reflection Runtime information about types C-Tracking-Issue An issue that collects information about a broad development initiative S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@aecsocket
Copy link
Contributor

aecsocket commented Sep 29, 2024

This is a meta-issue for tracking the progress of improving how reflect-deserialized assets get loaded. This is a cross-cutting concern between both reflection and assets.

The motivating use case comes from bevy_animation_graph, where we deserialize AnimationGraphs from RON files. This looks like:

#[derive(Asset, Reflect)]
struct AnimationGraph {
    pub nodes: Vec<AnimationNode>,
    pub edges: (...),
}

#[derive(Reflect)]
struct AnimationNode {
    pub name: String,
    pub inner: Box<dyn NodeLike>, // NodeLike: Reflect
}

#[derive(Reflect)]
#[reflect(NodeLike)]
struct AnimationClipNode {
    pub clip: Handle<AnimationClip>,
}

#[derive(Reflect)]
#[reflect(NodeLike)]
struct ChangeSpeedNode;

(
    nodes: [
        (
            name: "Walk clip",
            ty: "bevy_animation_graph::node::AnimationClipNode",
            inner: (
                clip: "animation_clips/walk.animclip.ron",
            )
        ),
        (
            name: "Change speed",
            ty: "bevy_animation_graph::node::ChangeSpeedNode",
            inner: (),
        ),
    ],
    edges: ( ... ),
)

The two important things here are:

  • AnimationNode stores a type-erased NodeLike
  • AnimationClipNode, and other node types, may store asset handles

However, it's currently very annoying to write code to deserialize and load this asset properly. Why?

If these issues are solved, we have a much more powerful way to easily define these kinds of complex assets which depend on other assets, and that also use reflection to deserialize values of unknown types.

Once these are resolved, I would like to write an example which deserializes an asset in this reflective way, showing off the automatic recursive Handle loading, and deserializing type-erased Box<dyn Reflect>s.

@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds A-Reflection Runtime information about types C-Tracking-Issue An issue that collects information about a broad development initiative labels Sep 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 17, 2024
**NOTE: This is based on, and should be merged alongside,
#15482 I'll leave this in
draft until that PR is merged.

# Objective

Equivalent of #15482 but for
serialization. See that issue for the motivation.

Also part of this tracking issue:
#15518

This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).

## Solution

Identical solution to the deserializer PR.

## Testing

Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
mockersf pushed a commit that referenced this issue Nov 17, 2024
**NOTE: This is based on, and should be merged alongside,
#15482 I'll leave this in
draft until that PR is merged.

# Objective

Equivalent of #15482 but for
serialization. See that issue for the motivation.

Also part of this tracking issue:
#15518

This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).

## Solution

Identical solution to the deserializer PR.

## Testing

Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
ecoskey pushed a commit to ecoskey/bevy that referenced this issue Dec 2, 2024
**NOTE: This is based on, and should be merged alongside,
bevyengine#15482 I'll leave this in
draft until that PR is merged.

# Objective

Equivalent of bevyengine#15482 but for
serialization. See that issue for the motivation.

Also part of this tracking issue:
bevyengine#15518

This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).

## Solution

Identical solution to the deserializer PR.

## Testing

Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
ecoskey pushed a commit to ecoskey/bevy that referenced this issue Jan 6, 2025
**NOTE: This is based on, and should be merged alongside,
bevyengine#15482 I'll leave this in
draft until that PR is merged.

# Objective

Equivalent of bevyengine#15482 but for
serialization. See that issue for the motivation.

Also part of this tracking issue:
bevyengine#15518

This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).

## Solution

Identical solution to the deserializer PR.

## Testing

Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
@BenjaminBrienen BenjaminBrienen added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Jan 7, 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 A-Reflection Runtime information about types C-Tracking-Issue An issue that collects information about a broad development initiative S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

3 participants