BlackPhlox / bevy_dolly

h3r2tic's dolly abstraction layer for the bevy game framework
Apache License 2.0
143 stars 22 forks source link

Use same entity for rig and camera #24

Closed janhohenheim closed 1 year ago

janhohenheim commented 1 year ago

It seems to me that the following kind of code...

    commands.spawn((
        MainCamera,
        Rig::builder()
            .with(Fpv::from_position_target(transform))
            .build(),
    ));

    commands.spawn((
        MainCamera,
        Camera3dBundle {
            transform,
            ..default()
        },
    ));

...could be more easily written as

    commands.spawn((
        MainCamera,
        Rig::builder()
            .with(Fpv::from_position_target(transform))
            .build(),
        Camera3dBundle {
            transform,
            ..default()
        },
    ));

From a quick check, it seems like the examples behave the same way, although this was not an extensive test. I assume that the design decision of not requiring this in the first place is to mix and match rigs and cameras, right?

BlackPhlox commented 1 year ago

Yep, looks much nicer, so I'm on board, you are welcome to add it to your change pr mentioned in #23

janhohenheim commented 1 year ago

If we consolidate this, can we remove the marker component in the same go? We can just query the (rig, camera) directly.

BlackPhlox commented 1 year ago

If we consolidate this, can we remove the marker component in the same go? We can just query the (rig, camera) directly.

I'm not sure what you mean here, we need the marker components (MainCamera) for Rig in order for the system to process the rig.

janhohenheim commented 1 year ago

I meant changing the system to query mut cameras: Query<(&mut Transform, &mut Rig, &Camera), Changed<Rig>>. Would that break anything?

BlackPhlox commented 1 year ago

Yes, cause it will crash at runtime if you have multiple rigs with different drivers and then a system calls rig.driver_mut::<Driver>() where the driver dosn't exist for that rig. However, now that I think about it, dolly does provides a try_driver_mut we could use instead, but then we have to force users to always use try_driver_mut. So it really is a question about api ergonomics.