[Merged by Bors] - Fix unsoundnes in insert remove and despawn

[BoxyUwU]

EntityMut::move_entity_from_remove had two soundness bugs:

• When removing the entity from the archetype, the swapped entity had its table row updated to the same as the removed entity's
• When removing the entity from the table, the swapped entity did not have its table row updated

BundleInsert::insert had two/three soundness bugs

• When moving an entity to a new archetype from an insert, the swapped entity had its table row set to a different entities
• When moving an entity to a new table from an insert, the swapped entity did not have its table row updated
  See added tests for examples that trigger those bugs

EntityMut::despawn had two soundness bugs

• When despawning an entity, the swapped entity had its table row set to a different entities even if the table didnt change
• When despawning an entity, the swapped entity did not have its table row updated

[james7132]

Suggested change

	archetype_id: swapped_location.archetype_id,
	archetype_row: old_location.archetype_row,
	table_id: swapped_location.table_id,
	table_row: swapped_location.table_row,
	archetype_row: old_location.archetype_row,
	..swapped_location

Alternatively, we should consider exposing a pub(crate) get_mut from Entities or just mutate swapped_location with the archetype row.

[BoxyUwU]

I specifically chose to construct a whole new struct here so that if we change the fields we'll get a compile error rather than it continuing to compile

[james7132]

That makes sense to me. Not sure if the compiler will optimize that down though. Probably worth investigating separately.

Not sure if this is worth leaving a comment so others don't try to make that change.

[james7132]

Suggested change

	archetype_id: swapped_location.archetype_id,
	archetype_row: swapped_location.archetype_row,
	table_id: swapped_location.table_id,
	table_row: old_location.table_row,
	table_row: old_location.table_row,
	..swapped_location

Alternatively just mutate the table_row on swapped_location.

[maniwani]

Insertion has the same UB, I think.

Overwrites table row.

bevy/crates/bevy_ecs/src/bundle.rs

Lines 556 to 559 in d1a1f90

	let result = self.archetype.swap_remove(location.archetype_row);
	if let Some(swapped_entity) = result.swapped_entity {
	self.entities.set(swapped_entity.index(), location);
	}

bevy/crates/bevy_ecs/src/bundle.rs

Lines 584 to 587 in d1a1f90

	let result = self.archetype.swap_remove(location.archetype_row);
	if let Some(swapped_entity) = result.swapped_entity {
	self.entities.set(swapped_entity.index(), location);
	}

Does not update table row on EntityLocation.

bevy/crates/bevy_ecs/src/bundle.rs

Lines 597 to 613 in d1a1f90

	if let Some(swapped_entity) = move_result.swapped_entity {
	let swapped_location = self.entities.get(swapped_entity).unwrap();
	let swapped_archetype = if self.archetype.id() == swapped_location.archetype_id
	{
	&mut *self.archetype
	} else if new_archetype.id() == swapped_location.archetype_id {
	new_archetype
	} else {
	// SAFETY: the only two borrowed archetypes are above and we just did collision checks
	&mut *self
	.archetypes_ptr
	.add(swapped_location.archetype_id.index())
	};
	swapped_archetype
	.set_entity_table_row(swapped_location.archetype_row, result.table_row);
	}

[james7132]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• msrv
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12