Repository: mlange-42/arche
Pull request #357 (2024-02-02 01:39:08)

Prevent using the same component multiple times in any operations, through panic
Disallows things like these:

```go
e := world.NewEntity(posID, posID) // add the same thing twice
world.Exchange(e, []ID{posID}, []ID{posID}) // add and remove the same thing
```

Which would have caused bugs when used.

coveralls: ## Pull Request Test Coverage Report for [Build 7750171565](https://coveralls.io/builds/65454944)


* **0** of **13**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **100.0%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/65454944/badge)](https://coveralls.io/builds/65454944) |
| :-- | --: |
| Change from base [Build 7750012433](https://coveralls.io/builds/65454692): |  0.0% |
| Covered Lines: | 6159 |
| Relevant Lines: | 6159 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 52ca113..4a4d769 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## [[unpublished]](https://github.com/mlange-42/arche/compare/v0.10.1...main)
+
+### Bugfixes
+
+* Prevents using the same component multiple times in any operations, through panic (#357)
+
 ## [[v0.10.1]](https://github.com/mlange-42/arche/compare/v0.10.0...v0.10.1)
 
 ### Bugfixes
diff --git a/ecs/world_internal.go b/ecs/world_internal.go
index 88c2fa3..b2e77fd 100644
--- a/ecs/world_internal.go
+++ b/ecs/world_internal.go
@@ -511,18 +511,18 @@ func (w *World) exchange(entity Entity, add []ID, rem []ID, relation ID, hasRela
 
 // Modify a mask by adding and removing IDs.
 func (w *World) getExchangeMask(mask Mask, add []ID, rem []ID) Mask {
-	for _, comp := range add {
-		if mask.Get(comp) {
-			panic(fmt.Sprintf("entity already has component of type %v, can't add", w.registry.Types[comp.id]))
-		}
-		mask.Set(comp, true)
-	}
 	for _, comp := range rem {
 		if !mask.Get(comp) {
 			panic(fmt.Sprintf("entity does not have a component of type %v, can't remove", w.registry.Types[comp.id]))
 		}
 		mask.Set(comp, false)
 	}
+	for _, comp := range add {
+		if mask.Get(comp) {
+			panic(fmt.Sprintf("entity already has component of type %v, can't add", w.registry.Types[comp.id]))
+		}
+		mask.Set(comp, true)
+	}
 	return mask
 }
 
@@ -897,6 +897,11 @@ func (w *World) findOrCreateArchetype(start *archetype, add []ID, rem []ID, targ
 	relation := start.RelationComponent
 	hasRelation := start.HasRelationComponent
 	for _, id := range rem {
+		// Not required, as removing happens only via exchange,
+		// which calls getExchangeMask, which does the same check.
+		//if !mask.Get(id) {
+		//	panic(fmt.Sprintf("entity does not have a component of type %v, or it was removed twice", w.registry.Types[id.id]))
+		//}
 		mask.Set(id, false)
 		if w.registry.IsRelation.Get(id) {
 			relation = ID{}
@@ -912,6 +917,12 @@ func (w *World) findOrCreateArchetype(start *archetype, add []ID, rem []ID, targ
 		}
 	}
 	for _, id := range add {
+		if mask.Get(id) {
+			panic(fmt.Sprintf("entity already has component of type %v, or it was added twice", w.registry.Types[id.id]))
+		}
+		if start.Mask.Get(id) {
+			panic(fmt.Sprintf("component of type %v added and removed in the same exchange operation", w.registry.Types[id.id]))
+		}
 		mask.Set(id, true)
 		if w.registry.IsRelation.Get(id) {
 			if hasRelation {


Test patch:
diff --git a/ecs/world_test.go b/ecs/world_test.go
index d31ae7f..e9ad02d 100644
--- a/ecs/world_test.go
+++ b/ecs/world_test.go
@@ -553,6 +553,19 @@ func TestWorldGetComponents(t *testing.T) {
 	assert.True(t, pos2 == nil)
 }
 
+func TestWorldDuplicateComponents(t *testing.T) {
+	w := NewWorld()
+
+	posID := ComponentID[Position](&w)
+	//rotID := ComponentID[rotation](&w)
+
+	assert.Panics(t, func() { w.NewEntity(posID, posID) })
+
+	e := w.NewEntity(posID)
+	assert.Panics(t, func() { w.Remove(e, posID, posID) })
+	assert.Panics(t, func() { w.Exchange(e, []ID{posID}, []ID{posID}) })
+}
+
 func TestWorldIter(t *testing.T) {
 	world := NewWorld()
 
@@ -1566,8 +1579,7 @@ func TestArchetypeGraph(t *testing.T) {
 	assert.Equal(t, int32(2), world.archetypes.Len())
 	assert.Equal(t, int32(4), world.nodes.Len())
 
-	arch01 := world.findOrCreateArchetype(arch0, []ID{velID}, []ID{}, Entity{})
-	arch012 := world.findOrCreateArchetype(arch01, []ID{rotID}, []ID{}, Entity{})
+	arch012 := world.findOrCreateArchetype(arch0, []ID{rotID}, []ID{}, Entity{})
 
 	assert.Equal(t, []ID{id(0), id(1), id(2)}, arch012.node.Ids)