Skip to content

feat: pre-register reflected components with the world at finalize #314

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

Merged
merged 6 commits into from
Feb 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions .github/workflows/bevy_mod_scripting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Rust Cache
uses: Swatinem/rust-cache@v2.7.7
with:
# reasoning: we want to cache xtask, most of the jobs in the matrix will be sped up a good bit thanks to that
save-if: ${{ github.ref == 'refs/heads/main' }}
cache-all-crates: true
- name: Generate matrix
id: generate-matrix
run: |
Expand Down Expand Up @@ -90,10 +96,13 @@ jobs:
with:
toolchain: stable
override: true

- name: Rust Cache
if: ${{ needs.check-needs-run.outputs.any-changes == 'true' }}
uses: Swatinem/rust-cache@v2.7.3
uses: Swatinem/rust-cache@v2.7.7
with:
# reasoning: we want to cache xtask, most of the jobs in the matrix will be sped up a good bit thanks to that
save-if: ${{ github.ref == 'refs/heads/main' }}
cache-all-crates: true

- name: Setup
if: ${{ needs.check-needs-run.outputs.any-changes == 'true' }}
Expand Down Expand Up @@ -143,6 +152,13 @@ jobs:
uses: actions/checkout@v4
with:
ref: ${{ github.head_ref || github.ref_name }}
- name: Rust Cache
if: ${{ needs.check-needs-run.outputs.any-changes == 'true' }}
uses: Swatinem/rust-cache@v2.7.7
with:
# reasoning: we want to cache xtask, most of the jobs in the matrix will be sped up a good bit thanks to that
save-if: ${{ github.ref == 'refs/heads/main' }}
cache-all-crates: true
- name: Setup Bot GitHub Credentials
run: |
git config user.name "github-actions[bot]"
Expand Down
36 changes: 36 additions & 0 deletions crates/bevy_mod_scripting_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,23 @@ fn once_per_app_finalize(app: &mut App) {
extensions: asset_settings_extensions,
preprocessor: None,
});

// pre-register component id's
pre_register_componnents(app);
}

/// Ensures all types with `ReflectComponent` type data are pre-registered with component ID's
fn pre_register_componnents(app: &mut App) {
let type_registry = app
.world_mut()
.get_resource_or_init::<AppTypeRegistry>()
.clone();
let type_registry = type_registry.read();

let world = app.world_mut();
for (_, data) in type_registry.iter_with_data::<ReflectComponent>() {
data.register_component(world);
}
}

// One of registration of things that need to be done only once per app
Expand Down Expand Up @@ -398,4 +415,23 @@ mod test {
.await
.expect("Rhai loader not found");
}

#[test]
fn test_reflect_component_is_preregistered_in_app_finalize() {
let mut app = App::new();

app.add_plugins(AssetPlugin::default());

#[derive(Component, Reflect)]
#[reflect(Component)]
struct Comp;

app.register_type::<Comp>();

assert!(app.world_mut().component_id::<Comp>().is_none());

once_per_app_finalize(&mut app);

assert!(app.world_mut().component_id::<Comp>().is_some());
}
}
30 changes: 29 additions & 1 deletion crates/xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ impl App {
cmd.arg("--coverage");
}

if let Some(jobs) = self.global_args.jobs {
cmd.arg("--jobs").arg(jobs.to_string());
}

match self.subcmd {
Xtasks::Macros { macro_name } => {
cmd.arg("macros").arg(macro_name.as_ref());
Expand Down Expand Up @@ -414,9 +418,24 @@ struct GlobalArgs {
help = "The cargo profile to use for commands that support it"
)]
profile: Option<String>,

#[clap(
long,
global = true,
value_name = "JOBS",
help = "The number of parallel jobs to run at most"
)]
jobs: Option<usize>,
}

impl GlobalArgs {
pub fn with_max_jobs(self, jobs: usize) -> Self {
Self {
jobs: Some(jobs),
..self
}
}

pub fn with_coverage(self) -> Self {
Self {
coverage: true,
Expand Down Expand Up @@ -809,6 +828,11 @@ impl Xtasks {
args.push("--profile".to_owned());
args.push(use_profile.to_owned());
}

if let Some(jobs) = app_settings.jobs {
args.push("--jobs".to_owned());
args.push(jobs.to_string());
}
}

args.extend(app_settings.features.to_cargo_args());
Expand Down Expand Up @@ -1344,7 +1368,11 @@ impl Xtasks {

// and finally run tests with coverage
output.push(App {
global_args: default_args.clone().with_coverage(),
global_args: default_args
.clone()
.with_coverage()
// github actions has been throwing a lot of OOM SIGTERM's lately
.with_max_jobs(4),
subcmd: Xtasks::Test {
name: None,
package: None,
Expand Down
Loading