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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Reflect Default for ComputedVisibility and Handle<T> #6187

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Reflect Default for ComputedVisibility and Handle<T> #6187

wants to merge 2 commits into from

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
@zicklag zicklag mentioned this pull request Oct 6, 2022
Closed
alice-i-cecile
alice-i-cecile reviewed Oct 6, 2022
View reviewed changes
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.

alice-i-cecile
alice-i-cecile approved these changes Oct 6, 2022
View reviewed changes
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
@zicklag
Reflect Default for ComputedVisibility and Handle<T> (#6187)
cfba731 
# 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

Pull request successfully merged into main.

Build succeeded:

@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
@zicklag zicklag mentioned this pull request Oct 8, 2022
Closed
bors bot pushed a commit that referenced this pull request Oct 10, 2022
@zicklag
Reflect Default for GlobalTransform (#6200)
a18e2b1 
# 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
@zicklag @james7132
Reflect Default for ComputedVisibility and Handle<T> (bevyengine#…
57b4f53 
…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
@zicklag @james7132
Reflect Default for GlobalTransform (bevyengine#6200)
7c09b27 
# 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
@zicklag @james7132
Reflect Default for ComputedVisibility and Handle<T> (bevyengine#…
3c577b6 
…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
@zicklag @james7132
Reflect Default for GlobalTransform (bevyengine#6200)
65edc7a 
# 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
@zicklag @Pietrek14
Reflect Default for GlobalTransform (bevyengine#6200)
e36b661 
# 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
@zicklag @ItsDoot
Reflect Default for ComputedVisibility and Handle<T> (bevyengine#…
0411d05 
…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
@zicklag @ItsDoot
Reflect Default for GlobalTransform (bevyengine#6200)
9e8c802 
# 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
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zicklag @alice-i-cecile @mockersf