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

Remove TypeRegistry re-export rename #9807

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

nicopap
Copy link
Contributor

@nicopap nicopap commented Sep 14, 2023

Objective

The rename is confusing. Each time I import TypeRegistry I have to think at least 10 seconds about how to import it. And I've been working a lot with bevy reflect, which multiplies the papercut.

In my crates, you can find lots of:

use bevy::reflect::{TypeRegistryInternal as TypeRegistry};

When I "go to definition" on TypeRegistry I get to TypeRegistryArc. And when I mean TypeRegistry in my function signature, 100% of the time I mean TypeRegistry, not the arc wrapper.

Rust has borrowing, and most use-cases of the TypeRegistry accepts borrow of the registry, with no need to mutate it.

TypeRegistryInternal is also confusing. In bevy crates, it doesn't exist. The bevy crate documentation often refers to TypeRegistry and link to TypeRegistryInternal. It only exists in the bevy re-exports. It makes it hard to understand which names qualifies which types.

Solution

Remove the rename, keep the type names as they are in bevy_reflect


Changelog

  • Remove TypeRegistry and TypeRegistryArc renames from bevy bevy_reflect re-exports.

Migration Guide

  • TypeRegistry as re-exported by the wrapper bevy crate is now TypeRegistryArc
  • TypeRegistryInternal as re-exported by the wrapper bevy crate is now TypeRegistry

The rename is confusing. Each time I import `TypeRegistry` I have to
think at least 10 seconds about how to import it. And I've been working
a lot of bevy reflect, which multiplies the papercut.

When I "go to definition" on `TypeRegistry` I get to `TypeRegistryArc`.
And when I mean `TypeRegistry` in my function signature,
100% of the time I mean `TypeRegistry`, not the arc wrapper.

Rust has borrowing, and most use-cases of the TypeRegistry
accepts borrow of the registry, with no need to mutate it.

`TypeRegistryInternal` is also confusing. In bevy crates,
it doesn't exist. It only exists in the bevy re-exports.
It makes it hard to understand which names qualifies which types.
@nicopap nicopap added A-Reflection Runtime information about types C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Sep 14, 2023
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

LGTM. Definitely stuck out like a sore thumb in the public interface.

@cart cart added this pull request to the merge queue Sep 14, 2023
Merged via the queue into bevyengine:main with commit 5e00379 Sep 14, 2023
24 checks passed
@nicopap nicopap deleted the no_type_registry_rename branch September 20, 2023 13:07
@djeedai
Copy link
Contributor

djeedai commented Sep 24, 2023

I lost 30 min yesterday trying to understand a compiler error due to this, with no help from documentation. Definitely a much welcome change, thanks! 😊

rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

The rename is confusing. Each time I import `TypeRegistry` I have to
think at least 10 seconds about how to import it. And I've been working
a lot with bevy reflect, which multiplies the papercut.

In my crates, you can find lots of:

```rust
use bevy::reflect::{TypeRegistryInternal as TypeRegistry};
```

When I "go to definition" on `TypeRegistry` I get to `TypeRegistryArc`.
And when I mean `TypeRegistry` in my function signature, 100% of the
time I mean `TypeRegistry`, not the arc wrapper.

Rust has borrowing, and most use-cases of the TypeRegistry accepts
borrow of the registry, with no need to mutate it.

`TypeRegistryInternal` is also confusing. In bevy crates, it doesn't
exist. The bevy crate documentation often refers to `TypeRegistry` and
link to `TypeRegistryInternal`. It only exists in the bevy re-exports.
It makes it hard to understand which names qualifies which types.

## Solution

Remove the rename, keep the type names as they are in `bevy_reflect`

---

## Changelog

- Remove `TypeRegistry` and `TypeRegistryArc` renames from bevy
`bevy_reflect` re-exports.

## Migration Guide

- `TypeRegistry` as re-exported by the wrapper `bevy` crate is now
`TypeRegistryArc`
- `TypeRegistryInternal` as re-exported by the wrapper `bevy` crate is
now `TypeRegistry`
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants