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

Alternative to remove_with_requires that doesn't remove components that still have other dependants #17757

Open
Zoomulator opened this issue Feb 9, 2025 · 1 comment
Labels
C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled

Comments

@Zoomulator
Copy link
Contributor

Zoomulator commented Feb 9, 2025

What problem does this solve or what need does it fill?

remove_with_requires currently makes no additional checks to see if any other component requires the same dependencies. This can result in the method removing another component's required dependency.

Take this example, where I expected A to remain because it is still required by C

#[derive(Component, Default)]
struct A;

#[derive(Component, Default)]
#[require(A)]
struct B;

#[derive(Component, Default)]
#[require(A)]
struct C;

#[test]
fn test_remove_double_requires() {
  let mut world = World::new();
  let mut entity = world.spawn((B, C));
  entity.remove_with_requires::<B>();
  assert!(entity.contains::<A>()); // fails
}

What solution would you like?

Let required components have a sort of ref-counting of required components. The current implementation of remove_with_requires could be altered, or a soft variant called something like remove_with_checked_requires could be added. This method would only remove a required component if no remaining components still requires it.

What alternative(s) have you considered?

It's possible to make your own function that makes sure to check if specific components are present before removing a dependency, and call that instead if you need to remove the required component.

@Zoomulator Zoomulator added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 9, 2025
@LorenVS
Copy link

LorenVS commented Feb 9, 2025

I think this issue raises some bigger questions about other APIs on EntityRef. For example, should we have a similar checked version of remove_by_id?

I spent some time looking into this today and ended up with LorenVS@4f83c59. The implementation in remove_bundle_from_archetype is very ugly, but I was just seeing whether I could get something implemented with the desired semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled
Projects
None yet
Development

No branches or pull requests

2 participants