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

[Merged by Bors] - bevy_reflect: Reflect arrays #4701

Closed
wants to merge 15 commits into from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented May 9, 2022

Objective

ℹ️ Note: This is a rebased version of #2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

Solution

  • Implement reflection for arrays via the Array trait.
  • Note, Array is different from List in the way that you cannot push elements onto an array as they are statically sized.
  • Now List is defined as a sub-trait of Array.

Changelog

  • Added the Array reflection trait
  • Allows arrays up to length 32 to be reflected via the Array trait

Migration Guide

  • The List trait now has the Array supertrait. This means that clone_dynamic will need to specify which version to use:
    // Before
    let cloned = my_list.clone_dynamic();
    // After
    let cloned = List::clone_dynamic(&my_list);
  • All implementers of List will now need to implement Array (this mostly involves moving the existing methods to the Array impl)

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Reflection Runtime information about types labels May 9, 2022
MrGVSV and others added 2 commits May 10, 2022 23:25
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@MrGVSV MrGVSV requested a review from alice-i-cecile May 11, 2022 06:28
Copy link
Contributor

@PROMETHIA-27 PROMETHIA-27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good aside from some nitpicks!

crates/bevy_reflect/src/serde/de.rs Outdated Show resolved Hide resolved
examples/reflection/reflection_types.rs Outdated Show resolved Hide resolved
MrGVSV and others added 2 commits May 12, 2022 12:50
Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 12, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 13, 2022
# Objective

> ℹ️ **Note**: This is a rebased version of #2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

- Currently reflection is not supported for arrays.
- Fixes #1213

## Solution

* Implement reflection for arrays via the `Array` trait.
* Note, `Array` is different from `List` in the way that you cannot push elements onto an array as they are statically sized.
* Now `List` is defined as a sub-trait of `Array`.

---

## Changelog

* Added the `Array` reflection trait
* Allows arrays up to length 32 to be reflected via the `Array` trait

## Migration Guide

* The `List` trait now has the `Array` supertrait. This means that `clone_dynamic` will need to specify which version to use:
  ```rust
  // Before
  let cloned = my_list.clone_dynamic();
  // After
  let cloned = List::clone_dynamic(&my_list);
  ```
* All implementers of `List` will now need to implement `Array` (this mostly involves moving the existing methods to the `Array` impl)

Co-authored-by: NathanW <nathansward@comcast.net>
Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
@bors bors bot changed the title bevy_reflect: Reflect arrays [Merged by Bors] - bevy_reflect: Reflect arrays May 13, 2022
@bors bors bot closed this May 13, 2022
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 17, 2022
# Objective

> ℹ️ **Note**: This is a rebased version of bevyengine#2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

- Currently reflection is not supported for arrays.
- Fixes bevyengine#1213

## Solution

* Implement reflection for arrays via the `Array` trait.
* Note, `Array` is different from `List` in the way that you cannot push elements onto an array as they are statically sized.
* Now `List` is defined as a sub-trait of `Array`.

---

## Changelog

* Added the `Array` reflection trait
* Allows arrays up to length 32 to be reflected via the `Array` trait

## Migration Guide

* The `List` trait now has the `Array` supertrait. This means that `clone_dynamic` will need to specify which version to use:
  ```rust
  // Before
  let cloned = my_list.clone_dynamic();
  // After
  let cloned = List::clone_dynamic(&my_list);
  ```
* All implementers of `List` will now need to implement `Array` (this mostly involves moving the existing methods to the `Array` impl)

Co-authored-by: NathanW <nathansward@comcast.net>
Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

> ℹ️ **Note**: This is a rebased version of bevyengine#2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

- Currently reflection is not supported for arrays.
- Fixes bevyengine#1213

## Solution

* Implement reflection for arrays via the `Array` trait.
* Note, `Array` is different from `List` in the way that you cannot push elements onto an array as they are statically sized.
* Now `List` is defined as a sub-trait of `Array`.

---

## Changelog

* Added the `Array` reflection trait
* Allows arrays up to length 32 to be reflected via the `Array` trait

## Migration Guide

* The `List` trait now has the `Array` supertrait. This means that `clone_dynamic` will need to specify which version to use:
  ```rust
  // Before
  let cloned = my_list.clone_dynamic();
  // After
  let cloned = List::clone_dynamic(&my_list);
  ```
* All implementers of `List` will now need to implement `Array` (this mostly involves moving the existing methods to the `Array` impl)

Co-authored-by: NathanW <nathansward@comcast.net>
Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
bors bot pushed a commit that referenced this pull request Sep 2, 2022
# Objective

The documentation on `Reflect` doesn't account for the recently added reflection traits: [`Array`](#4701) and [`Enum`](#4761).

## Solution

Updated the documentation for `Reflect` to account for the `Array` and `Enum`.


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

The documentation on `Reflect` doesn't account for the recently added reflection traits: [`Array`](bevyengine#4701) and [`Enum`](bevyengine#4761).

## Solution

Updated the documentation for `Reflect` to account for the `Array` and `Enum`.


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

The documentation on `Reflect` doesn't account for the recently added reflection traits: [`Array`](bevyengine#4701) and [`Enum`](bevyengine#4761).

## Solution

Updated the documentation for `Reflect` to account for the `Array` and `Enum`.


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

> ℹ️ **Note**: This is a rebased version of bevyengine#2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

- Currently reflection is not supported for arrays.
- Fixes bevyengine#1213

## Solution

* Implement reflection for arrays via the `Array` trait.
* Note, `Array` is different from `List` in the way that you cannot push elements onto an array as they are statically sized.
* Now `List` is defined as a sub-trait of `Array`.

---

## Changelog

* Added the `Array` reflection trait
* Allows arrays up to length 32 to be reflected via the `Array` trait

## Migration Guide

* The `List` trait now has the `Array` supertrait. This means that `clone_dynamic` will need to specify which version to use:
  ```rust
  // Before
  let cloned = my_list.clone_dynamic();
  // After
  let cloned = List::clone_dynamic(&my_list);
  ```
* All implementers of `List` will now need to implement `Array` (this mostly involves moving the existing methods to the `Array` impl)

Co-authored-by: NathanW <nathansward@comcast.net>
Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
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-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement Reflect for arrays
4 participants