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

Moves intern and label modules into bevy_ecs #12772

Merged
merged 6 commits into from
Apr 8, 2024

Conversation

mnmaita
Copy link
Member

@mnmaita mnmaita commented Mar 29, 2024

Objective

Solution

  • Moved intern module from bevy_utils into bevy_ecs crate and updated all relevant imports.
  • Moved label module from bevy_utils into bevy_ecs crate and updated all relevant imports.

Migration Guide

  • Replace bevy_utils::define_label imports with bevy_ecs::define_label imports.
  • Replace bevy_utils::label::DynEq imports with bevy_ecs::label::DynEq imports.
  • Replace bevy_utils::label::DynHash imports with bevy_ecs::label::DynHash imports.
  • Replace bevy_utils::intern::Interned imports with bevy_ecs::intern::Interned imports.
  • Replace bevy_utils::intern::Internable imports with bevy_ecs::intern::Internable imports.
  • Replace bevy_utils::intern::Interner imports with bevy_ecs::intern::Interner imports.

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide A-Utils Utility functions and types labels Mar 30, 2024
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.

Mind fixing CI? The lints seem like they should be fixable by using cast::<()> instead of as.

@mnmaita mnmaita force-pushed the mnmaita/move-intern-and-label branch from 3aea575 to c67e12a Compare March 30, 2024 12:03
@mnmaita mnmaita marked this pull request as ready for review March 30, 2024 12:10
@mnmaita
Copy link
Member Author

mnmaita commented Mar 30, 2024

Mind fixing CI? The lints seem like they should be fixable by using cast::<()> instead of as.

Done, rebased and updated the migration guide. Feel free to take a second look at it now @james7132. Thanks!

@mnmaita mnmaita requested a review from james7132 March 30, 2024 12:11
crates/bevy_ecs/src/label.rs Outdated Show resolved Hide resolved
@mnmaita mnmaita force-pushed the mnmaita/move-intern-and-label branch 2 times, most recently from 5635dfb to 5a2ebf3 Compare April 1, 2024 23:11
@mnmaita
Copy link
Member Author

mnmaita commented Apr 4, 2024

@james7132 I did a change that fixed the issue with Interned in the docs, but now there's a different one... Could you please help me figure out why Schedule is giving errors during doc generation? I don't really know what's the cause of this issue. Thanks!

system::{
ExclusiveFunctionSystem, ExclusiveSystemParamFunction, FunctionSystem,
IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction,
},
};

define_label!(
Copy link
Member

Choose a reason for hiding this comment

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

I think you just need to make this [`Schedule`](crate::schedule::Schedule).

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be fixed in the latest change @james7132

@mnmaita mnmaita force-pushed the mnmaita/move-intern-and-label branch from 5a2ebf3 to 6b030bc Compare April 7, 2024 10:50
@mnmaita mnmaita requested a review from james7132 April 7, 2024 15:27
@james7132 james7132 added this to the 0.14 milestone Apr 8, 2024
@james7132 james7132 added this pull request to the merge queue Apr 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 8, 2024
@mnmaita
Copy link
Member Author

mnmaita commented Apr 8, 2024

@james7132 something failed during the merge queuing process (and I see no logs on the step that failed... Maybe it was just a blip in the matrix during artifact upload?). I've updated the branch, if you could queue this one again I'll appreciate it.

@james7132 james7132 added this pull request to the merge queue Apr 8, 2024
Merged via the queue into bevyengine:main with commit 0c78bf3 Apr 8, 2024
27 checks passed
@mnmaita mnmaita deleted the mnmaita/move-intern-and-label branch April 9, 2024 14:37
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 A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide 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.

2 participants