Skip to content

Conversation

@zicklag
Copy link
Member

@zicklag zicklag commented Oct 6, 2022

Objective

  • Reflecting Default is required for scripts to create Reflect types at runtime with no static type information.
  • Reflecting Default on Handle<T> and ComputedVisibility should allow scripts from bevy_mod_js_scripting to actually spawn sprites from scratch, without needing any hand-holding from the host-game.

Solution

  • Derive ReflectDefault for Handle<T> and ComputedVisiblity.

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

  • The Default trait is now reflected for Handle<T> and ComputedVisibility

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types labels Oct 6, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Shouldn't we have a Default trait impl for this reflection to work? Or at least for consistency.

@zicklag
Copy link
Member Author

zicklag commented Oct 6, 2022

Yes, both types already had Default implementations, they just aren't derived, they're implemented manually.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation; it wasn't visible in the code review pane :)

@alice-i-cecile
Copy link
Member

Merging as trivial.

bors r+

bors bot pushed a commit that referenced this pull request Oct 6, 2022
# Objective

- Reflecting `Default` is required for scripts to create `Reflect` types at runtime with no static type information.
- Reflecting `Default` on `Handle<T>` and `ComputedVisibility` should allow scripts from `bevy_mod_js_scripting` to actually spawn sprites from scratch, without needing any hand-holding from the host-game.

## Solution

- Derive `ReflectDefault` for `Handle<T>` and `ComputedVisiblity`.

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- The `Default` trait is now reflected for `Handle<T>` and `ComputedVisibility`
@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 Oct 6, 2022
@bors
Copy link
Contributor

bors bot commented Oct 6, 2022

@bors bors bot changed the title Reflect Default for ComputedVisibility and Handle<T> [Merged by Bors] - Reflect Default for ComputedVisibility and Handle<T> Oct 6, 2022
@bors bors bot closed this Oct 6, 2022
bors bot pushed a commit that referenced this pull request Oct 10, 2022
# Objective

Make `GlobalTransform` constructible from scripts, in the same vein as #6187.

## Solution

- Use the derive macro to reflect default

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- `GlobalTransform` now reflects the `Default` trait.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
…6187)

# Objective

- Reflecting `Default` is required for scripts to create `Reflect` types at runtime with no static type information.
- Reflecting `Default` on `Handle<T>` and `ComputedVisibility` should allow scripts from `bevy_mod_js_scripting` to actually spawn sprites from scratch, without needing any hand-holding from the host-game.

## Solution

- Derive `ReflectDefault` for `Handle<T>` and `ComputedVisiblity`.

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- The `Default` trait is now reflected for `Handle<T>` and `ComputedVisibility`
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
# Objective

Make `GlobalTransform` constructible from scripts, in the same vein as bevyengine#6187.

## Solution

- Use the derive macro to reflect default

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- `GlobalTransform` now reflects the `Default` trait.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…6187)

# Objective

- Reflecting `Default` is required for scripts to create `Reflect` types at runtime with no static type information.
- Reflecting `Default` on `Handle<T>` and `ComputedVisibility` should allow scripts from `bevy_mod_js_scripting` to actually spawn sprites from scratch, without needing any hand-holding from the host-game.

## Solution

- Derive `ReflectDefault` for `Handle<T>` and `ComputedVisiblity`.

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- The `Default` trait is now reflected for `Handle<T>` and `ComputedVisibility`
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

Make `GlobalTransform` constructible from scripts, in the same vein as bevyengine#6187.

## Solution

- Use the derive macro to reflect default

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- `GlobalTransform` now reflects the `Default` trait.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
# Objective

Make `GlobalTransform` constructible from scripts, in the same vein as bevyengine#6187.

## Solution

- Use the derive macro to reflect default

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- `GlobalTransform` now reflects the `Default` trait.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…6187)

# Objective

- Reflecting `Default` is required for scripts to create `Reflect` types at runtime with no static type information.
- Reflecting `Default` on `Handle<T>` and `ComputedVisibility` should allow scripts from `bevy_mod_js_scripting` to actually spawn sprites from scratch, without needing any hand-holding from the host-game.

## Solution

- Derive `ReflectDefault` for `Handle<T>` and `ComputedVisiblity`.

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- The `Default` trait is now reflected for `Handle<T>` and `ComputedVisibility`
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Make `GlobalTransform` constructible from scripts, in the same vein as bevyengine#6187.

## Solution

- Use the derive macro to reflect default

---

## Changelog

> This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

- `GlobalTransform` now reflects the `Default` trait.
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-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants