rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Dedicated 2D Transform

Open BobG1983 opened this issue 1 year ago • 9 comments

The ergonomics of using Bevy's existing Transform for 2D games is poor. To counter this a 2D Transform component should be created that has improved ergonomics.

Rendered: https://github.com/BobG1983/bevy_rfcs/blob/main/rfcs/82-transform2d.md

BobG1983 avatar Jun 01 '24 23:06 BobG1983

Oh, and one last thing. I want to mix 2d and 3d transforms can be mixed seamlessly in the hierarchy; mixing 2d and 3d stuff like this in games is super common. This will need to have an expanded section on the way these should interact.

NthTensor avatar Jun 04 '24 14:06 NthTensor

Yes. I think there's a clear consensus that something like this is needed, and the maintenance burden is just the cost of properly supporting 2d (which is one of bevy's core goals).

I agree

Are we happy about using a fairly simple/naive DrawOrder implementation?

I am not, and I suspect I will not be until I see some prototypes. This is not just a question of ergonomics, but also of correctness, flexibility, and performance. It's really difficult to assess these without actually writing and evaluating several z-layering systems.

Professionally, I've used Z (Transform), seperate draw layer (f32), and "define list of draw layers and pick". Either of the 2nd two is fine IMO, I've no preference. If you have other suggestions I'd love to hear them, but I find the f32 approach the simplest and least "magic".

What are the knock-on effects on UI and its desire for a potential UiTransform/TransformUi

It's vitally important that bevy UI not use these 2d transforms. I think UI deserves it's own layout/transform types which are informed by this work, but which exist in "camera space" rather than "world space" and so will not trip up crates likebig_space.

Agreed.

BobG1983 avatar Jun 04 '24 15:06 BobG1983

If you have other suggestions I'd love to hear them, but I find the f32 approach the simplest and least "magic".

See my rough proposal here. This issue also has a really good list of criteria for the layering system.

I am most worried about making users worry about manual reshuffling to make room for new intermediate layers. If reshuffling is necessary, I think bevy should try to handle it quietly by default. While we are laying out our preferences, I would strongly prefer using numerics (floating points or integers) over using enums or discrete types for layers/ordering.

NthTensor avatar Jun 04 '24 15:06 NthTensor

So your suggestion is the user sets up layers, that have an f32 depth. Then you assign a sprite to a layer?

Layers are relational? Above/below?

The inner functionality of those layers can then be whatever (your proposal if f32) and we handle relayering "magically" when a new layer is added?

BobG1983 avatar Jun 04 '24 18:06 BobG1983

As a user, it would make sense to me to go with DrawOrder for the context of this RFC. It seems about the same as current behavior, which let's this RFC focus on reworking the API vs figuring out a new feature. 1275 could be an RFC / working group on its own and I'd rather have Transform2d in one release and a layering system in another than wait for both.

MScottMcBee avatar Jun 05 '24 14:06 MScottMcBee

I agree, incrementalism would be wise here.

NthTensor avatar Jun 05 '24 16:06 NthTensor

I took a stab at implementing this RFC in a fork and got the Breakout example to work with just Transform2ds. I'd like to share some thoughts and further questions I ran into.

I think, for now, DrawLayer should be part of Transform2d. It doesn't make sense for a Transform2d to not have a DrawLayer to me. Until required components or an equivalent construct comes online having them separate sounds like it's just asking for users to miss it. I think it's worth having it be its own type rather than a raw f32 to leave room for implementing a more involved layering system later.

I don't know if I agree with having Sprite2dBundle and Sprite3dBundle. At least, I think SpriteBundle should stay and use Transform2d. It might be breaking but I think it's more intuitive

The approach outlined here is nice, but I'm not 100% sold on it. There are two large questions that make me say that.

  1. Should hierarchies mix? Should EntityA with Transform3d be able to have EntityB with a Transform2d as a child? If not, how an we enforce that?

If they can, do they effect each other or is it organizational?

If they can effect each other, what does that even mean? Would EntityB be layered according to EntityA's Z value, or would all sprites be drawn on top of the 3d scene, only using the screen space X & Y from EntityA?

If EntityB is layered into the 3d scene, is that better served with a billboard component that has a Trasnsform3d instead? If it's drawn on top, would it be better served with a different mechanism than parenting with different transform components?

In my branch I modified sync_simple_transforms to work on both Transform3d and Transform2d. It's ugly.

pub fn sync_simple_transforms(
    mut query: ParamSet<(
        Query<
            (
                Option<&Transform3d>,
                Option<&Transform2d>,
                &mut GlobalTransform,
            ),
            (
                Or<(Changed<Transform3d>, Changed<Transform2d>, Added<GlobalTransform>)>,
                Without<Parent>,
                Without<Children>,
                Or<(With<Transform3d>, With<Transform2d>)>,
            ),
        >,
        Query<
            (
                Option<Ref<Transform3d>>,
                Option<Ref<Transform2d>>,
                &mut GlobalTransform,
            ),
            (
                Without<Parent>,
                Without<Children>,
                Or<(With<Transform3d>, With<Transform2d>)>,
            ),
        >,
    )>,
    mut orphaned: RemovedComponents<Parent>,
) {
    // Update changed entities.
    query.p0().par_iter_mut().for_each(
        |(opt_transform3d, opt_transform2d, mut global_transform)| {
            *global_transform = opt_transform3d
                .map(|t| GlobalTransform::from(*t))
                .or(opt_transform2d.map(|t| GlobalTransform::from(*t)))
                .unwrap();
        },
    );
    // Update orphaned entities.
    let mut query = query.p1();
    let mut iter = query.iter_many_mut(orphaned.read());
    while let Some((opt_transform3d, opt_transform2d, mut global_transform)) = iter.fetch_next() {
        if let Some(transform) = opt_transform3d {
            if !transform.is_changed() && !global_transform.is_added() {
                *global_transform = GlobalTransform::from(*transform);
            }
        }
        if let Some(transform) = opt_transform2d {
            if !transform.is_changed() && !global_transform.is_added() {
                *global_transform = GlobalTransform::from(*transform);
            }
        }
    }
}

I'll concede I'm not the most experienced Rust/Bevy dev, so maybe this can be done better, but I'm really not a fan of the ergonomics of having multiple optionals in a query. It's not TOO bad for two transform types, but what if we want something like TransformUI or TransformUI3d later on?

  1. What if an entity has multiple Transform components? This would leave which one ends up resulting into the GlobalTransform to system ambiguities. But how would we stop that? I don't know of any way to set components to be exclusive. We could have a system to remove one component if the other was added, but it'd be nice to have something at compile time.

MScottMcBee avatar Jun 26 '24 18:06 MScottMcBee

Perhaps the answer is that we don't have transform as a component at all but a query struct. Then the individual components are split out and the only actual Tranform becomes a compute LocalTransform and a computed GlobalTransform.

I think mixed transform hierarchies is an interesting question. I don't have an opinion.

In terms of DrawOrder not being a raw f32. I'm fine with having a type though we may want to start it as a new type of f32 or a deref f32 now.

FYI @NthTensor who is planning on putting a working group together.

BobG1983 avatar Jul 07 '24 20:07 BobG1983

I dont think a seperate transform component is the way to go. What should happen when an entity has both Transform and Transform2d? What about systems that just use transform, e.g. the transform propagation. You don't want to write them twice. We could make transform an enum, for example

enum Transform {
    2d {
        position: Vec2,
        depth: f32,
        rotation: f32,
        scale: Vec2,
    },
    3d {
        translation: Vec3,
        rotation: Quat,
        scale: Vec3,
    },
}

But here we also would have to split the logic twice, plus having a breaking change for every 3d user. When we now reextract the two possibilities into seperate structs and add the methods .2d() and .3d() to it, we would still have a breaking change, because now everyone has to do .3d(), but all the other problems should be solved. Then the question is if we always store the 3d transform, or store an enum with the dimension from initialization and just return directly or convert on the fly when called.

DasLixou avatar Aug 02 '24 11:08 DasLixou