added serde(default) to Transform

[GageHowe]

Objective

Need serde(Default) for Transform. I think it would be very useful in cases where someone wants to add an object to their scene but doesn't want to have to specify rotation or scale, e.g.:

5: (
  components: {
    "common::types::GameObjectKind": Rifle,
    "bevy_transform::components::transform::Transform": (
      translation: (1.5, 800.0, 0.0),
    ),
  },
),

Solution

Added #[cfg_attr(feature = "serialize", serde(default))] before the Transform struct.

Testing

Successfully built bevy, but haven't tested extensively. Theoretically shouldn't break existing functionality, since Deriving Transform was invalid before this PR.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨

[alice-i-cecile]

Deriving Transform was invalid before this PR

I think this was a typo, but I can't figure out what you mean here.

I think this is a reasonable idea, but I'd be more comfortable with this if there was tests in place for the desired functionality.

[GageHowe]

I think this was a typo, but I can't figure out what you mean here.

Just that omitting, say, scale, was considered invalid and would be a ron parsing error, so I don't imagine this would break any existing functionality.

I'm a bit new. How should I go about making tests?

[alice-i-cecile]

So, for unit tests in Rust use should make a mod tests module inside of the file that you're testing.

Then, inside of that module, you can define a test function, annotated with #[test].

In this case, we should be testing round-trip serialization with partially and fully omitted fields. @andriyDev may have more advice about the best way to do this.

Here's an example of a similar test from one of my crates: https://github.com/Leafwing-Studios/leafwing_manifest/blob/f5a2da1c742babe14d3c4816d4769bfa145e3b47/examples/raw_manifest.rs#L164

I think you should be able to get away without writing it to disk, and should prefer doing so if possible. Storing the string in memory for serialization-deserialization should work fine.

[GageHowe]

How's this? All tests pass with cargo test -p bevy_transform --features serialize

[alice-i-cecile]

Good, thanks for the tests. @cart, how does stuff like this align with your vision of .bsn assets? I know you have similar "fill in the blank" functionality intended there.

[cart]

I'm on board for this / see no harm in it. I view it as being similar, but orthogonal to the BSN design, which should not be using serde for this.

[cart]

We should remove these tests. They are testing ron's implementation and usage of serde(default), not code that we own or maintain. This is a waste of compute resources that could be spent elsewhere (or not spent at all).

[GageHowe]

@alice-i-cecile thoughts?

[alice-i-cecile]

Sure, we can cut them.