From bc9b8063b5ba480bd089ac003e5950896425a17d Mon Sep 17 00:00:00 2001 From: Robert Gardner Date: Sun, 26 May 2024 20:03:02 -0700 Subject: [PATCH 1/6] Empty new rfc --- rfcs/split_transform.md | 82 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 rfcs/split_transform.md diff --git a/rfcs/split_transform.md b/rfcs/split_transform.md new file mode 100644 index 00000000..f6de76fc --- /dev/null +++ b/rfcs/split_transform.md @@ -0,0 +1,82 @@ +# Feature Name: (fill me in with a unique ident, `my_awesome_feature`) + +## Summary + +One paragraph explanation of the feature. + +## Motivation + +Why are we doing this? What use cases does it support? + +## User-facing explanation + +Explain the proposal as if it was already included in the engine and you were teaching it to another Bevy user. That generally means: + +- Introducing new named concepts. +- Explaining the feature, ideally through simple examples of solutions to concrete problems. +- Explaining how Bevy users should *think* about the feature, and how it should impact the way they use Bevy. It should explain the impact as concretely as possible. +- If applicable, provide sample error messages, deprecation warnings, or migration guidance. +- If applicable, explain how this feature compares to similar existing features, and in what situations the user would use each one. + +## Implementation strategy + +This is the technical portion of the RFC. +Try to capture the broad implementation strategy, +and then focus in on the tricky details so that: + +- Its interaction with other features is clear. +- It is reasonably clear how the feature would be implemented. +- Corner cases are dissected by example. + +When necessary, this section should return to the examples given in the previous section and explain the implementation details that make them work. + +When writing this section be mindful of the following [repo guidelines](https://github.com/bevyengine/rfcs): + +- **RFCs should be scoped:** Try to avoid creating RFCs for huge design spaces that span many features. Try to pick a specific feature slice and describe it in as much detail as possible. Feel free to create multiple RFCs if you need multiple features. +- **RFCs should avoid ambiguity:** Two developers implementing the same RFC should come up with nearly identical implementations. +- **RFCs should be "implementable":** Merged RFCs should only depend on features from other merged RFCs and existing Bevy features. It is ok to create multiple dependent RFCs, but they should either be merged at the same time or have a clear merge order that ensures the "implementable" rule is respected. + +## Drawbacks + +Why should we *not* do this? + +## Rationale and alternatives + +- Why is this design the best in the space of possible designs? +- What other designs have been considered and what is the rationale for not choosing them? +- What objections immediately spring to mind? How have you addressed them? +- What is the impact of not doing this? +- Why is this important to implement as a feature of Bevy itself, rather than an ecosystem crate? + +## \[Optional\] Prior art + +Discuss prior art, both the good and the bad, in relation to this proposal. +This can include: + +- Does this feature exist in other libraries and what experiences have their community had? +- Papers: Are there any published papers or great posts that discuss this? + +This section is intended to encourage you as an author to think about the lessons from other tools and provide readers of your RFC with a fuller picture. + +Note that while precedent set by other engines is some motivation, it does not on its own motivate an RFC. + +## Unresolved questions + +- What parts of the design do you expect to resolve through the RFC process before this gets merged? +- What parts of the design do you expect to resolve through the implementation of this feature before the feature PR is merged? +- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? + +## \[Optional\] Future possibilities + +Think about what the natural extension and evolution of your proposal would +be and how it would affect Bevy as a whole in a holistic way. +Try to use this section as a tool to more fully consider other possible +interactions with the engine in your proposal. + +This is also a good place to "dump ideas", if they are out of scope for the +RFC you are writing but otherwise related. + +Note that having something written down in the future-possibilities section +is not a reason to accept the current or a future RFC; such notes should be +in the section on motivation or rationale in this or subsequent RFCs. +If a feature or change has no direct value on its own, expand your RFC to include the first valuable feature that would build on it. From 80c809e7adb1a0f3e627501cf0298ca9352ff172 Mon Sep 17 00:00:00 2001 From: Robert Gardner Date: Sat, 1 Jun 2024 13:56:10 -0700 Subject: [PATCH 2/6] Accurately named --- rfcs/{split_transform.md => transform2d.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rfcs/{split_transform.md => transform2d.md} (100%) diff --git a/rfcs/split_transform.md b/rfcs/transform2d.md similarity index 100% rename from rfcs/split_transform.md rename to rfcs/transform2d.md From 4e7990d0947f93ba4e40cab5bd9eb6d781bdecb7 Mon Sep 17 00:00:00 2001 From: Robert Gardner Date: Sat, 1 Jun 2024 15:30:58 -0700 Subject: [PATCH 3/6] First version of transform2d rfc --- rfcs/transform2d.md | 157 ++++++++++++++++++++++++++++++-------------- 1 file changed, 108 insertions(+), 49 deletions(-) diff --git a/rfcs/transform2d.md b/rfcs/transform2d.md index f6de76fc..8ad1c74a 100644 --- a/rfcs/transform2d.md +++ b/rfcs/transform2d.md @@ -1,82 +1,141 @@ -# Feature Name: (fill me in with a unique ident, `my_awesome_feature`) +# Dedicated 2D Transform ## Summary -One paragraph explanation of the feature. +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. ## Motivation -Why are we doing this? What use cases does it support? +While it is readily possible to create 2D games in Bevy today, interacting with the existing Transform system is awkward, high in "ceremony", and prone to unintentional human error. There are two main issues: + +1. Translation is represented as a `Vec3` with the draw order set implicity based on the Z value +2. Rotation being represented as a Quat even though all rotations are around the Z axis + +### The Problem with Translation + +Translation in Bevy is stored as a `Vec3` in the Transform component. This is fine for 3D games, and is usable for 2D games, but has several issues. + +#### Overloading the Concept + +In 2D games this `Vec3` represents two seperate concepts - the 2D position (X,Y) of the entity, and the draw order (Z) of the entity. Vector maths, thus, can end up changing the draw order value when done on the full `Vec3`, leading to graphical issues being caused by systems that seem unrelated to graphics. This can be easy to forget, and difficult to debug. Avoiding this requires writing a lot boilerplate, which leads to... + +#### Poor Ergonomics + +In order to avoid issues caused by the above overloading any system that interacts with an entities translation will often store the entities Z, copy a `Vec2` of the translation using either xy() or truncate(), perform any necessary operations, then re-extend the `Vec2` back to a `Vec3` with the stored Z before reassigning the value to the Transforms translation. The effect of this is that a 2D games code is littered with truncate() and extend(). This is then further exasperated when doing operations like Raycasting where maintaining the existing Z value will give incorrect results (including Z distance in ray calculations). All in all, this is a poor user experience. + +### The Problem with Rotation + +Rotation in bevy is stored as a `Quat` in the Transform component. Again, this is fine for 3D games, and is usable for 2D games, but has poor ergonomics. Rotation in a 2D game in Bevy is always counterclockwise around the Z axis. As with using `Vec3`'s for Translation this causes a lot of boilerplate, extracting the value of a quaternions rotation around Z in radians, performing calculations on that value, then creating a new `Quat` rotating around Z before reassigning. Much as with Translation, this is a poor user experience. ## User-facing explanation -Explain the proposal as if it was already included in the engine and you were teaching it to another Bevy user. That generally means: +Transform2D represents a 2D entities position, rotation, and scale in world space. It is defined as follows: -- Introducing new named concepts. -- Explaining the feature, ideally through simple examples of solutions to concrete problems. -- Explaining how Bevy users should *think* about the feature, and how it should impact the way they use Bevy. It should explain the impact as concretely as possible. -- If applicable, provide sample error messages, deprecation warnings, or migration guidance. -- If applicable, explain how this feature compares to similar existing features, and in what situations the user would use each one. +```rust +pub struct Transform2d { + position: Vec2, + rotation: f32 //RFC Note: We could use Rotation2d here + scale: Vec2 +} +``` + +If you're making a 2D game and you want your entity to appear in the world at a given location it must have a `Transform2D` component. `Position` represents your entities location in the world (actually it's position on the 'xy-plane'). `Rotation` represents your entities anti-clockwise rotation in radians (around the Z-axis). `Scale` represents the entities scale in the world (like `Position` this is actually the entities scale on the 'xy-plane'). + +Many entities in a 2D game will have a sprite, a mesh, or some other graphics attached to them. By default, all these entities are drawn on the same 'layer'. When entities are drawn the last entity drawn appears on top of thosed drawn before it in the same location. Developers can control which layer an entity is on by changing that entities `DrawLayer`: + +```rust +pub struct DrawLayer(pub f32); +``` + +As you can see `DrawLayer` is just a wrapper around an `f32`. Entities with a higher value for their `DrawLayer` will be drawn on top of those with lower values. ## Implementation strategy -This is the technical portion of the RFC. -Try to capture the broad implementation strategy, -and then focus in on the tricky details so that: +### Components + +#### Transform + +A `Transform2d` component should be created as follows: + +```rust +pub struct Transform2d { + position: Vec2, + rotation: f32 //Note: Optionally a Rotation2d + scale: Vec2 +} +``` + +The existing `Transform` component should be be kept as is to maintain backwards compatibility. + +(Note: If we don't care about said backwards compatibility I would choose to rename `Transform` to `Transform3d`) -- Its interaction with other features is clear. -- It is reasonably clear how the feature would be implemented. -- Corner cases are dissected by example. +#### DrawOrder -When necessary, this section should return to the examples given in the previous section and explain the implementation details that make them work. +A `DrawOrder` component should be created as follows: -When writing this section be mindful of the following [repo guidelines](https://github.com/bevyengine/rfcs): +```rust +pub struct DrawLayer(pub f32); +``` +For entities with parent/child relationships, child entities should have their `DrawOrder` propogated to an offset from their parents `DrawOrder` as with the existing `Transform` component. We should not do anything more than this (ie. we should not attempt to automatically set child entities to some lower value of `DrawOrder` than their parent). -- **RFCs should be scoped:** Try to avoid creating RFCs for huge design spaces that span many features. Try to pick a specific feature slice and describe it in as much detail as possible. Feel free to create multiple RFCs if you need multiple features. -- **RFCs should avoid ambiguity:** Two developers implementing the same RFC should come up with nearly identical implementations. -- **RFCs should be "implementable":** Merged RFCs should only depend on features from other merged RFCs and existing Bevy features. It is ok to create multiple dependent RFCs, but they should either be merged at the same time or have a clear merge order that ensures the "implementable" rule is respected. +### Bundles + +#### Spatial2dBundle + +A new spatial bundle should be created as per the existing `SpatialBundle`, but containing a `Transform2d` instead of a `Transform`. + +#### SpriteBundle/SpriteSheetBundle + +Two new bundles should be created `Sprite2dBundle` and `SpriteSheet2dBundle` matching the existing `SpriteBundle` and `SpriteSheetBundle` except replacing `Transform` with `Transform2d`. + +The existing `SpriteBundle` and `SpriteSheetBundle` should be kept as is to maintain backwards compatibility. + +(Note: If we don't care about said backwards compatibility I would choose to rename the existing bundles to `Sprite3dBundle` and `SpriteSheet3dBundle`) + +### Systems + +New versions of `sync_simple_transforms`, `propogate_transforms`, and `propogate_transforms_recursive` should be written that propogate the new `Transform2d` into the existing `GlobalTransform`. ## Drawbacks -Why should we *not* do this? +### Maintenance Burden -## Rationale and alternatives +Beyond having three new systems and some new components to maintain Bevy as a whole would need to choose to either: -- Why is this design the best in the space of possible designs? -- What other designs have been considered and what is the rationale for not choosing them? -- What objections immediately spring to mind? How have you addressed them? -- What is the impact of not doing this? -- Why is this important to implement as a feature of Bevy itself, rather than an ecosystem crate? +* Offer `Vec2` versions of many of it's `Vec3` functions, potentially labelling as eg. `RayCast2d` vs `RayCast` +* Require developers to call `extend()` and use the `Vec3` versions of these functions. Which may defy the point of doing this in the first place. -## \[Optional\] Prior art +This burden is unlikely to go away once this decision is made, for the lifetime of the engine. -Discuss prior art, both the good and the bad, in relation to this proposal. -This can include: +### Ecosystem Pressure -- Does this feature exist in other libraries and what experiences have their community had? -- Papers: Are there any published papers or great posts that discuss this? +As above, many ecosystem crates (`bevy_xpbd`, `bevy_rapier`, for example) would need to answer the same question. Either offering `Vec2` versions or requiring use of `extend()`. In some cases these crates already do this, but this would 'force the question'. -This section is intended to encourage you as an author to think about the lessons from other tools and provide readers of your RFC with a fuller picture. +### One Way Door? -Note that while precedent set by other engines is some motivation, it does not on its own motivate an RFC. +Once this door is passed through, it may be difficult to walk back, its likely that UI will want similar versions of `Transform` for their needs and ecosystem crates would likely start to support `Transform2d`. Once this change is made, it is likely made permanently. -## Unresolved questions +## Rationale and alternatives + +There are really only two options: + +* Have a `Tranform2d` component +* Don't have a `Transform2d` component -- What parts of the design do you expect to resolve through the RFC process before this gets merged? -- What parts of the design do you expect to resolve through the implementation of this feature before the feature PR is merged? -- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? +The existing system has poor ergonomics, not making this change will continue to favor the development of 3D games, make 2D games more difficult to develop and more prone to error. I believe this may, in the long term, harm uptake of the engine given the large proportion of indie games are 2D and Indie devs are more likely to risk using a new 'less proven' tech stack like Bevy and Rust. -## \[Optional\] Future possibilities +For `DrawOrder` we could have a specific layer system, where a user registers draw layers, and selects from a set of pre-registered draw layers, but this doesn't handle relative positioning. We could handle relative positioning in parent/child relationships by associating some float range and sharing the range between children but many developers would need to manually work around this in order to have children appear on the layers they want (see the existing demand for better control of transform propogation). + +In regards to support burden, this support burden must 'be paid' either way, either in the engine, or in the game. Right now we require that burden to be paid by the game developer. I believe it is more correct for that support burden to be paid by the engine and reduce the burden on the game developer. + +In regards to the ecosystem effect, right now functionality like this already exists in several crates. There is no standardized solution, and developers would need to know of these possible solutions before starting - something they're unlikely to know given they are unlikely to be embedded in the ecosystem. A standardized in engine solution would improve user experience, improve indie uptake, and - I believe - improve the crate ecosystem by allowing for better targetting of 2D specific crates. + +## Unresolved questions -Think about what the natural extension and evolution of your proposal would -be and how it would affect Bevy as a whole in a holistic way. -Try to use this section as a tool to more fully consider other possible -interactions with the engine in your proposal. +- Do we want to take this burden ourselves? +- Are we happy about using a fairly simple/naive `DrawOrder` implementation? +- What are the knock-on effects on UI and its desire for a potential `UiTransform`/`TransformUi` -This is also a good place to "dump ideas", if they are out of scope for the -RFC you are writing but otherwise related. +## Future possibilities -Note that having something written down in the future-possibilities section -is not a reason to accept the current or a future RFC; such notes should be -in the section on motivation or rationale in this or subsequent RFCs. -If a feature or change has no direct value on its own, expand your RFC to include the first valuable feature that would build on it. +I have my own crate `rantz_spatial2d` which I'm using in my own game. It's far from perfect and poorly documented, but in it, I have split `Transform2d` into its component parts of `Position2D`, `Rotation2D`, and `Scale2D`. It's out of the scope of this RFC, but often, in 2D games, you actually don't want the whole Transform, only a segment of it, and it may be beneficial to consider splitting `Transform2d` and `Transform` in the same way at some point. Again, outside the scope of this RFC, but an interesting thought. From 66442c5f34f8a4ac8b4c2e2880ee847ba4e04dee Mon Sep 17 00:00:00 2001 From: Robert Gardner Date: Sat, 1 Jun 2024 15:40:35 -0700 Subject: [PATCH 4/6] Added deprecated note --- rfcs/transform2d.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/rfcs/transform2d.md b/rfcs/transform2d.md index 8ad1c74a..64d4bb2c 100644 --- a/rfcs/transform2d.md +++ b/rfcs/transform2d.md @@ -65,9 +65,7 @@ pub struct Transform2d { } ``` -The existing `Transform` component should be be kept as is to maintain backwards compatibility. - -(Note: If we don't care about said backwards compatibility I would choose to rename `Transform` to `Transform3d`) +The existing `Transform` component should annotated as `[deprecated]` and a copy created named `Transform3d`. #### DrawOrder @@ -88,9 +86,7 @@ A new spatial bundle should be created as per the existing `SpatialBundle`, but Two new bundles should be created `Sprite2dBundle` and `SpriteSheet2dBundle` matching the existing `SpriteBundle` and `SpriteSheetBundle` except replacing `Transform` with `Transform2d`. -The existing `SpriteBundle` and `SpriteSheetBundle` should be kept as is to maintain backwards compatibility. - -(Note: If we don't care about said backwards compatibility I would choose to rename the existing bundles to `Sprite3dBundle` and `SpriteSheet3dBundle`) +The existing `SpriteBundle` and `SpriteSheetBundle` should be annotated as `[Deprecated]` and copies created named `Sprite3dBundle` and `SpriteSheet3dBundle`. ### Systems From 3e52496dca4f46d8bf66cc5f3a8aaa785e1a5424 Mon Sep 17 00:00:00 2001 From: Robert Gardner Date: Sat, 1 Jun 2024 16:25:16 -0700 Subject: [PATCH 5/6] Adding PR number --- rfcs/{transform2d.md => 82-transform2d.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rfcs/{transform2d.md => 82-transform2d.md} (100%) diff --git a/rfcs/transform2d.md b/rfcs/82-transform2d.md similarity index 100% rename from rfcs/transform2d.md rename to rfcs/82-transform2d.md From 1bca0164efc627b59bcc8789985d6f7c94356cad Mon Sep 17 00:00:00 2001 From: Bob Gardner Date: Tue, 4 Jun 2024 08:19:26 -0700 Subject: [PATCH 6/6] Update 82-transform2d.md with rotation2d for transform2d rotation --- rfcs/82-transform2d.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/82-transform2d.md b/rfcs/82-transform2d.md index 64d4bb2c..7fa3a4fb 100644 --- a/rfcs/82-transform2d.md +++ b/rfcs/82-transform2d.md @@ -34,7 +34,7 @@ Transform2D represents a 2D entities position, rotation, and scale in world spac ```rust pub struct Transform2d { position: Vec2, - rotation: f32 //RFC Note: We could use Rotation2d here + rotation: Rotation2d scale: Vec2 } ```