Skip to content

Move Name from bevy_core to bevy_ecs #4842

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

Closed
wants to merge 1 commit into from
Closed

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented May 26, 2022

Objective

  • Part of breaking bevy_core up into more specific locations.
  • Name is universally applicable to ecs consumers, so should be available in bevy_ecs.

Solution

  • Move Name from bevy_core to bevy_ecs.
  • Name utilizes #[derive(Component)]; use extern crate self as bevy_ecs to make the expected name available.

Changelog

Changed

  • The Name struct moved from bevy::core to bevy::ecs.

Migration Guide

  • If you're using a bevy::prelude import, no migration is necessary.
  • If you aren't, replace uses of bevy::core::Name with bevy::ecs::Name.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events A-Core X-Controversial There is active debate or serious implications around merging this PR C-Code-Quality A section of code that is hard to understand or change labels May 26, 2022
@cart
Copy link
Member

cart commented May 26, 2022

Name is a pretty opinionated component. It prehashes strings, uses a "fast compare" based on that, and uses a Cow internally. It is also a "generic" Name type, which outside the context of a general purpose engine / scene system (or games with some use for a generic Name construct) probably isn't advisable.

At least for me, merging this would be motivated largely by a desire to remove bevy_core and less by a conviction that bevy_ecs is the place for it.

Given that for Bevy, this is intended to be used for editor / scene scenarios, I think bevy_scene might be the better place for it. It exists "higher up" in the stack and I think that wouldn't require changing our dependency graph at all.

@CAD97
Copy link
Contributor Author

CAD97 commented May 26, 2022

Makes sense! I'll retool this to move it to bevy_scene later then.

@CAD97 CAD97 closed this May 26, 2022
@CAD97 CAD97 deleted the name-is-ecs branch May 26, 2022 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants