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

Rename many types to better reflect their usability (in the context of Components and Resources) #11772

Closed
wants to merge 1 commit into from

Conversation

Adamkob12
Copy link
Contributor

@Adamkob12 Adamkob12 commented Feb 7, 2024

Objective

Solution

  • Rename many types that had the word Component in the them to a name without the word Component (usually Data) to reflect that they could be used in the context of resources as well.

Changelog

  • Rename Components to WorldData
  • Rename ComponentId to DataId
  • Rename ComponentDescriptor to DataDescriptor
  • Rename ComponentInfo to DataInfo
  • Rename some local variables in different methods to reflect these changes (use component_id, data_id and resource_id appropriately based on the context)
  • Rename components field in World to world_data

Migration Guide

  • Use WorldData instead of Components
  • Use DataId instead of ComponentId
  • Use DataDescriptor instead of ComponentDescriptor
  • Use DataInfo instead of ComponentInfo

Additional Context

At first I wanted to f i x #3007 in one PR like #4955 tried but I realized that all of the refactoring and the renaming will make it very difficult to review. So I decided to split them out. This PR will be focused on renaming and iterating over all of the Components and Resources will be a different PR.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Feb 7, 2024
@Adamkob12 Adamkob12 changed the title Implement methods to list all components and all resources (Rebase of PR #4955) Rename many types to better reflect their usability (in the context of Components and Resources) Feb 8, 2024
@Adamkob12 Adamkob12 marked this pull request as ready for review February 8, 2024 14:32
@JoJoJet
Copy link
Member

JoJoJet commented Feb 12, 2024

I disagree with renaming Component to Data everywhere. It's more vague and doesn't communicate any specific meaning. Component meanings something specific within the ECS, and resources are just a special type of a component. It might be a bit counterintuitive at first, but that's okay because users never have to think about ComponentIds unless they want to. I'd rather have a distinct and meaningful name than a bunch of types named Data.

@JoJoJet JoJoJet added the X-Controversial There is active debate or serious implications around merging this PR label Feb 12, 2024
@Adamkob12
Copy link
Contributor Author

I'm not too attached to either.
On one hand, using ComponentId and the like for resources is needlessly confusing (I remember I had had trouble with it)
One the other hand, it's actually somewhat informative regarding how Bevy handles resources.

I also worry that if another type would be added in the future (I vaguely remember hearing talk of Tag?) it would make it worse.

Another option is encapsulating Data stuff in a trait, and keep ComponentId(+) and maybe add ResourceId(+)
but that's probably too much.

@coreh
Copy link
Contributor

coreh commented Feb 20, 2024

I think Data sounds perhaps too generic, which might make it confusing? There's also the problem of pluralization, since data is already plural, so you can't pluralize it further.

What about:

  • Rename Components to Representations
  • Rename ComponentId to RepresentationId
  • Rename ComponentDescriptor to RepresentationDescriptor
  • Rename ComponentInfo to RepresentationInfo
  • Rename components field in World to representations

@Adamkob12 Adamkob12 closed this Apr 11, 2024
@Adamkob12 Adamkob12 deleted the rename_comp_id_etc branch April 11, 2024 13:07
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-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

4 participants