Skip to content

Commit 13f826d

Browse files
authored
Undo split out column layout (#472)
1 parent a3dcbc8 commit 13f826d

File tree

16 files changed

+199
-163
lines changed

16 files changed

+199
-163
lines changed

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
# Changelog
22

3-
## [[unpublished]](https://github.com/mlange-42/ark/compare/v0.7.0...main)
3+
## [[v0.7.1]](https://github.com/mlange-42/ark/compare/v0.7.0...v0.7.1)
4+
5+
### Bugfixes
6+
7+
- Fixes non-nil component retrieved from mapper (#472, fixes #470, rolls back #430)
48

59
### Other
610

benchmark/table/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/mlange-42/ark/benchmark"
1111
)
1212

13-
const version = "v0.7.1-dev"
13+
const version = "v0.7.1"
1414
const goVersion = "1.25.4"
1515

1616
func main() {

ecs/column.go

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,14 @@ import (
77

88
// column storage for components in an table.
99
type column struct {
10-
columnLayout
11-
data reflect.Value // data buffer
12-
elemType reflect.Type // element type of the column
13-
target Entity // target entity if for a relation component
14-
index uint32 // index of the column in the containing table
15-
isRelation bool // whether this column is for a relation component
16-
isTrivial bool // Whether the column's type is trivial , i.e. without pointers.
17-
}
18-
19-
type columnLayout struct {
20-
pointer unsafe.Pointer // pointer to the first element
21-
itemSize uintptr // memory size of items
10+
data reflect.Value // data buffer
11+
pointer unsafe.Pointer // pointer to the first element
12+
itemSize uintptr // memory size of items
13+
elemType reflect.Type // element type of the column
14+
target Entity // target entity if for a relation component
15+
index uint32 // index of the column in the containing table
16+
isRelation bool // whether this column is for a relation component
17+
isTrivial bool // Whether the column's type is trivial , i.e. without pointers.
2218
}
2319

2420
// newColumn creates a new column for a given type and capacity.
@@ -28,10 +24,8 @@ func newColumn(index uint32, tp reflect.Type, itemSize uintptr, isRelation bool,
2824
pointer := data.Addr().UnsafePointer()
2925

3026
return column{
31-
columnLayout: columnLayout{
32-
pointer: pointer,
33-
itemSize: itemSize,
34-
},
27+
pointer: pointer,
28+
itemSize: itemSize,
3529
target: target,
3630
index: index,
3731
data: data,
@@ -42,7 +36,7 @@ func newColumn(index uint32, tp reflect.Type, itemSize uintptr, isRelation bool,
4236
}
4337

4438
// Get returns a pointer to the component at the given index.
45-
func (c *columnLayout) Get(index uintptr) unsafe.Pointer {
39+
func (c *column) Get(index uintptr) unsafe.Pointer {
4640
return unsafe.Add(c.pointer, index*c.itemSize)
4741
}
4842

ecs/internal/generate/maps.go.template

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,9 @@ func (m *Map{{.}}{{$genericsShort}}) Set(entity Entity, {{$args}}) {
270270
{{- end}}
271271

272272
index := &m.world.storage.entities[entity.id]
273+
row := uintptr(index.row)
273274
{{- range $i, $v := $upper}}
274-
set(m.storage{{$v}}, index, {{index $lower $i}})
275+
*(*{{$v}})(m.storage{{$v}}.columns[index.table].Get(row)) = *{{index $lower $i}}
275276
{{- end}}
276277

277278
if m.world.storage.observers.HasObservers(OnSetComponents) {

ecs/internal/generate/observers.go.template

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,11 @@ func (o *Observer{{.}}{{$genericsShort}}) Register(w *World) *Observer{{.}}{{$ge
121121
{{- end}}
122122
o.observer.callback = func(e Entity) {
123123
index := &w.storage.entities[e.id]
124+
row := uintptr(index.row)
124125
o.callback(
125126
e,
126127
{{- range $i, $v := $upper}}
127-
get[{{$v}}](storage{{$v}}, index),
128+
(*{{$v}})(storage{{$v}}.columns[index.table].Get(row)),
128129
{{- end}}
129130
)
130131
}

ecs/map.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (m *Map[T]) Set(entity Entity, comp *T) {
225225
m.world.storage.checkHasComponent(entity, m.ids[0])
226226

227227
index := &m.world.storage.entities[entity.id]
228-
set(m.storage, index, comp)
228+
*(*T)(m.storage.columns[index.table].Get(uintptr(index.row))) = *comp
229229

230230
if m.world.storage.observers.HasObservers(OnSetComponents) {
231231
newMask := &m.world.storage.archetypes[m.world.storage.tables[index.table].archetype].mask

ecs/map_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,44 @@ func TestMapAddRelationBatch(t *testing.T) {
384384
expectEqual(t, n, cnt)
385385
}
386386

387+
func TestMapGetReturnsNilForMissingComponentIssue470(t *testing.T) {
388+
world := NewWorld()
389+
posMap := NewMap[Position](world)
390+
velMap := NewMap[Velocity](world)
391+
392+
entity := posMap.NewEntity(&Position{X: 10, Y: 20})
393+
if !posMap.Has(entity) {
394+
t.Fatal("Entity should have Position component")
395+
}
396+
if velMap.Has(entity) {
397+
t.Fatal("Entity should NOT have Velocity component")
398+
}
399+
400+
vel := velMap.Get(entity)
401+
if vel != nil {
402+
t.Fatal("Velocity component should be nil")
403+
}
404+
}
405+
406+
func TestMap1GetReturnsNilForMissingComponentIssue470(t *testing.T) {
407+
world := NewWorld()
408+
posMap := NewMap1[Position](world)
409+
velMap := NewMap1[Velocity](world)
410+
411+
entity := posMap.NewEntity(&Position{X: 10, Y: 20})
412+
if !posMap.HasAll(entity) {
413+
t.Fatal("Entity should have Position component")
414+
}
415+
if velMap.HasAll(entity) {
416+
t.Fatal("Entity should NOT have Velocity component")
417+
}
418+
419+
vel := velMap.Get(entity)
420+
if vel != nil {
421+
t.Fatal("Velocity component should be nil")
422+
}
423+
}
424+
387425
func BenchmarkMap1Get_1000(b *testing.B) {
388426
w := NewWorld()
389427
mapper := NewMap1[Position](w)

ecs/maps_gen.go

Lines changed: 90 additions & 78 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)