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

Changes Reflect derive for Handle<T>, so scenes with Vec<Handle<T>> can be spawned #2216

Closed
wants to merge 1 commit into from

Conversation

lassade
Copy link
Contributor

@lassade lassade commented May 18, 2021

Simplest way of address #2215, but doesn't solve the problem completely

@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior labels May 18, 2021
@ValorZard
Copy link

Has anyone taken a look at this yet? it seems like a easy code review

@cart
Copy link
Member

cart commented May 20, 2021

Ive glanced at it. Its easy to review the code, but I'll need to play around with some scenes first to prove that it behaves the way it should. Planning on doing that today or tomorrow.

@cart
Copy link
Member

cart commented May 20, 2021

This breaks normal Handle<T> serialization / deserialization:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: Message("Type \'bevy_asset::handle::Handle<bevy_pbr::material::StandardMaterial>\' does not support ReflectValue serialization"), position: Position { line: 0, col: 0 } }', examples/scene/scene.rs:106:53

This is because Handle doesn't implement serde::Serialize and serde::Deserialize (which ReflectValue requires).

I also expect it to fail in the same way for Vecs.

Additionally, reflect_value does not support #[reflect(ignore)], so if we ever make this work that will need to be adjusted to ignore the fields using serde attributes.

@cart
Copy link
Member

cart commented May 20, 2021

Check out the other bevy ReflectValue impls for how this is done.

I modified the scene example to test this PR:

use bevy::{prelude::*, reflect::{TypeRegistry, TypeUuid}, utils::Duration};

/// This example illustrates loading and saving scenes from files
fn main() {
    App::build()
        .add_plugins(DefaultPlugins)
        .register_type::<ComponentA>()
        .register_type::<ComponentB>()
        .add_startup_system(save_scene_system.exclusive_system())
        .add_startup_system(load_scene_system.system())
        .add_startup_system(infotext_system.system())
        .add_system(log_system.system())
        .run();
}

// Registered components must implement the `Reflect` and `FromResources` traits.
// The `Reflect` trait enables serialization, deserialization, and dynamic property access.
// `Reflect` enable a bunch of cool behaviors, so its worth checking out the dedicated `reflect.rs`
// example. The `FromResources` trait determines how your component is constructed when it loads.
// For simple use cases you can just implement the `Default` trait (which automatically implements
// FromResources). The simplest registered component just needs these two derives:
#[derive(Reflect, Default)]
#[reflect(Component)] // this tells the reflect derive to also reflect component behaviors
struct ComponentA {
    pub x: f32,
    pub y: f32,
}

// Some components have fields that cannot (or should not) be written to scene files. These can be
// ignored with the #[reflect(ignore)] attribute. This is also generally where the `FromResources`
// trait comes into play. `FromResources` gives you access to your App's current ECS `Resources`
// when you construct your component.
#[derive(Reflect)]
#[reflect(Component)]
struct ComponentB {
    pub value: String,
    #[reflect(ignore)]
    pub _time_since_startup: Duration,
}

impl FromWorld for ComponentB {
    fn from_world(world: &mut World) -> Self {
        let time = world.get_resource::<Time>().unwrap();
        ComponentB {
            _time_since_startup: time.time_since_startup(),
            value: "Default Value".to_string(),
        }
    }
}

fn load_scene_system(asset_server: Res<AssetServer>, mut scene_spawner: ResMut<SceneSpawner>) {
    // Scenes are loaded just like any other asset.
    let scene_handle: Handle<DynamicScene> = asset_server.load("scenes/load_scene_example.scn.ron");

    // SceneSpawner can "spawn" scenes. "Spawning" a scene creates a new instance of the scene in
    // the World with new entity ids. This guarantees that it will not overwrite existing
    // entities.
    scene_spawner.spawn_dynamic(scene_handle);

    // This tells the AssetServer to watch for changes to assets.
    // It enables our scenes to automatically reload in game when we modify their files
    asset_server.watch_for_changes().unwrap();
}

// This system logs all ComponentA components in our world. Try making a change to a ComponentA in
// load_scene_example.scn. You should immediately see the changes appear in the console.
fn log_system(query: Query<(Entity, &ComponentA, &Handle<StandardMaterial>), Changed<ComponentA>>) {
    for (entity, component_a, material) in query.iter() {
        info!("  Entity({})", entity.id());
        info!("  Material: {:?}", material);
        info!(
            "    ComponentA: {{ x: {} y: {} }}\n",
            component_a.x, component_a.y
        );
    }
}

pub const MATERIAL_HANDLE: HandleUntyped =
    HandleUntyped::weak_from_u64(StandardMaterial::TYPE_UUID, 3234320022263993879);

fn save_scene_system(world: &mut World) {
    {
        let mut materials = world.get_resource_mut::<Assets<StandardMaterial>>().unwrap();
        materials.set_untracked(MATERIAL_HANDLE, StandardMaterial::default());
    };
    // Scenes can be created from any ECS World. You can either create a new one for the scene or
    // use the current World.
    let mut scene_world = World::new();
    let mut component_b = ComponentB::from_world(world);
    component_b.value = "hello".to_string();
    scene_world.spawn().insert_bundle((
        component_b,
        ComponentA { x: 1.0, y: 2.0 },
        Transform::identity(),
        MATERIAL_HANDLE.typed::<StandardMaterial>()
    ));
    scene_world
        .spawn()
        .insert_bundle((ComponentA { x: 3.0, y: 4.0 },));

    // The TypeRegistry resource contains information about all registered types (including
    // components). This is used to construct scenes.
    let type_registry = world.get_resource::<TypeRegistry>().unwrap();
    let scene = DynamicScene::from_world(&scene_world, &type_registry);

    // Scenes can be serialized like this:
    info!("{}", scene.serialize_ron(&type_registry).unwrap());

    // TODO: save scene
}

// This is only necessary for the info message in the UI. See examples/ui/text.rs for a standalone
// text example.
fn infotext_system(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn_bundle(UiCameraBundle::default());
    commands.spawn_bundle(TextBundle {
        style: Style {
            align_self: AlignSelf::FlexEnd,
            ..Default::default()
        },
        text: Text::with_section(
            "Nothing to see in this window! Check the console output!",
            TextStyle {
                font: asset_server.load("fonts/FiraSans-Bold.ttf"),
                font_size: 50.0,
                color: Color::WHITE,
            },
            Default::default(),
        ),
        ..Default::default()
    });
}

@cart cart changed the title Changes Reflect derive for Handle<T>, so scenes can be spawned Changes Reflect derive for Handle<T>, so scenes with Vec<Handle<T>> can be spawned May 20, 2021
@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@alice-i-cecile
Copy link
Member

Fixed by #1395 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants