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

Reflect: Allow to override the type name #3327

Closed
jcornaz opened this issue Dec 14, 2021 · 4 comments
Closed

Reflect: Allow to override the type name #3327

jcornaz opened this issue Dec 14, 2021 · 4 comments
Labels
A-Reflection Runtime information about types C-Usability A simple quality-of-life change that makes Bevy easier to use

Comments

@jcornaz
Copy link
Contributor

jcornaz commented Dec 14, 2021

What problem does this solve or what need does it fill?

As a plugin author I publish components and implement Reflect for them so that the consumers of my plugin can use them in serialized scenes.

Theses components may live in private modules and be re-exported from a parent module like this:

mod my_private_module {
  #[derive(Debug, Copy, Clone, Default, Component, Reflect]
  #[reflect(Component)]
  pub struct AwesomeComponent;
}

// Expose the component without the user knowing about `my_private_module`
pub use my_private_module::AwesomeComponent;

This as the advantage that moving the component to another module or rename/move the module is not a breaking change for the consumers of my library.

But when serialized via bevy's reflection system, it exposes the real path of the component. So moving my Component is now a breaking change. And that's annoying.

What solution would you like?

I would like to be able to choose (hard-code) the path of my components. Maybe like this:

#[derive(Debug, Copy, Clone, Default, Component, Reflect]
#[reflect(Component)]
#[reflect(rename = "myplugin::AwesomeComponent")] // <-- Choose a type name
pub struct AwesomeComponent;;

What alternative(s) have you considered?

I could not move my components (annoying for me). Or I could publish a breaking change (annoying for my users).

Additional context

Serde has a somewhat similar feature:

@jcornaz jcornaz added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Dec 14, 2021
@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types C-Usability A simple quality-of-life change that makes Bevy easier to use and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Dec 14, 2021
@MrGVSV
Copy link
Member

MrGVSV commented Oct 15, 2022

Would this be resolved by either #5805 or #5830?

@jcornaz
Copy link
Contributor Author

jcornaz commented Oct 15, 2022

Yes, I think #5805 would solve it. (If I understood well 😄)

@MrGVSV MrGVSV mentioned this issue Oct 16, 2022
5 tasks
@soqb soqb mentioned this issue Jan 13, 2023
2 tasks
alice-i-cecile pushed a commit that referenced this issue Jun 5, 2023
# Objective

- Introduce a stable alternative to
[`std::any::type_name`](https://doc.rust-lang.org/std/any/fn.type_name.html).
- Rewrite of #5805 with heavy inspiration in design.
- On the path to #5830.
- Part of solving #3327.


## Solution

- Add a `TypePath` trait for static stable type path/name information.
- Add a `TypePath` derive macro.
- Add a `impl_type_path` macro for implementing internal and foreign
types in `bevy_reflect`.

---

## Changelog

- Added `TypePath` trait.
- Added `DynamicTypePath` trait and `get_type_path` method to `Reflect`.
- Added a `TypePath` derive macro.
- Added a `bevy_reflect::impl_type_path` for implementing `TypePath` on
internal and foreign types in `bevy_reflect`.
- Changed `bevy_reflect::utility::(Non)GenericTypeInfoCell` to
`(Non)GenericTypedCell<T>` which allows us to be generic over both
`TypeInfo` and `TypePath`.
- `TypePath` is now a supertrait of `Asset`, `Material` and
`Material2d`.
- `impl_reflect_struct` needs a `#[type_path = "..."]` attribute to be
specified.
- `impl_reflect_value` needs to either specify path starting with a
double colon (`::core::option::Option`) or an `in my_crate::foo`
declaration.
- Added `bevy_reflect_derive::ReflectTypePath`.
- Most uses of `Ident` in `bevy_reflect_derive` changed to use
`ReflectTypePath`.

## Migration Guide

- Implementors of `Asset`, `Material` and `Material2d` now also need to
derive `TypePath`.
- Manual implementors of `Reflect` will need to implement the new
`get_type_path` method.

## Open Questions
- [x] ~This PR currently does not migrate any usages of
`std::any::type_name` to use `bevy_reflect::TypePath` to ease the review
process. Should it?~ Migration will be left to a follow-up PR.
- [ ] This PR adds a lot of `#[derive(TypePath)]` and `T: TypePath` to
satisfy new bounds, mostly when deriving `TypeUuid`. Should we make
`TypePath` a supertrait of `TypeUuid`? [Should we remove `TypeUuid` in
favour of
`TypePath`?](https://github.com/bevyengine/bevy/pull/5805/files/2afbd855327c4b68e0a6b6f03118f289988441a4#r961067892)
@nakedible
Copy link
Contributor

@alice-i-cecile I think this is solved now?

@alice-i-cecile
Copy link
Member

Agreed.

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 simple quality-of-life change that makes Bevy easier to use
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants