From 971a1bb8836284d15a1009f87cc2d2d98eb17dc1 Mon Sep 17 00:00:00 2001 From: Scott Date: Tue, 28 Jul 2026 16:58:45 -0700 Subject: [PATCH 1/4] feat(cardinal): allow private system state --- pkg/cardinal/system.go | 10 +++-- pkg/cardinal/system_internal_test.go | 58 ++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/pkg/cardinal/system.go b/pkg/cardinal/system.go index e5b3f5b0d..086b8db59 100644 --- a/pkg/cardinal/system.go +++ b/pkg/cardinal/system.go @@ -79,9 +79,13 @@ func initSystemFields[T any](state *T, world *World) error { field := value.Field(i) fieldType := value.Type().Field(i) - // If the field is not exported, return an error. - if !field.CanAddr() { - return eris.Errorf("field %s must be exported", fieldType.Name) + // Ignore private implementation state, but keep private Cardinal dependencies + // as fail-fast configuration errors. + if !fieldType.IsExported() { + if field.Addr().Type().Implements(reflect.TypeFor[systemField]()) { + return eris.Errorf("field %s must be exported", fieldType.Name) + } + continue } fieldInstance := field.Addr().Interface() diff --git a/pkg/cardinal/system_internal_test.go b/pkg/cardinal/system_internal_test.go index 17f4532b1..371137461 100644 --- a/pkg/cardinal/system_internal_test.go +++ b/pkg/cardinal/system_internal_test.go @@ -16,6 +16,64 @@ import ( // TODO: test system registration, e.g. duplicate field detection, etc. +type privateStateSystem struct { + BaseSystemState + + dependency *int + scratch []int +} + +type privateDependencySystem struct { + BaseSystemState + + events WithEvent[testutils.SimpleEvent] +} + +func TestRegisterSystem_AllowsPersistentPrivateState(t *testing.T) { + t.Parallel() + + dependency := 40 + world := &World{world: ecs.NewWorld()} + var firstState *privateStateSystem + + RegisterSystem(world, func(state *privateStateSystem) { + if state.dependency == nil { + state.dependency = &dependency + } + if firstState == nil { + firstState = state + } + + assert.Same(t, firstState, state) + (*state.dependency)++ + state.scratch = append(state.scratch, *state.dependency) + }) + world.world.Init() + world.world.Tick() + world.world.Tick() + + require.NotNil(t, firstState) + assert.Same(t, world, firstState.world) + assert.Equal(t, 42, dependency) + assert.Equal(t, []int{41, 42}, firstState.scratch) +} + +func TestRegisterSystem_RejectsPrivateCardinalDependency(t *testing.T) { + t.Parallel() + + world := &World{world: ecs.NewWorld()} + + require.PanicsWithError( + t, + "error initializing system fields: field events must be exported", + func() { + RegisterSystem(world, func(state *privateDependencySystem) { + _ = state.events + }) + }, + ) +} + // ------------------------------------------------------------------------------------------------- // WithCommand smoke tests // ------------------------------------------------------------------------------------------------- From 043ee8a479544c47b5473212ef3954a1958845fa Mon Sep 17 00:00:00 2001 From: sms-yui <287818108+sms-yui@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:40:41 +0000 Subject: [PATCH 2/4] feat(cardinal): add struct system registration --- pkg/cardinal/system.go | 89 ++++++++++++--- pkg/cardinal/system_internal_test.go | 161 +++++++++++++++++++++++---- 2 files changed, 214 insertions(+), 36 deletions(-) diff --git a/pkg/cardinal/system.go b/pkg/cardinal/system.go index 086b8db59..bc3a60140 100644 --- a/pkg/cardinal/system.go +++ b/pkg/cardinal/system.go @@ -19,6 +19,13 @@ import ( type EntityID = ecs.EntityID +// System is a stateful Cardinal system. Implement it with a Run method on a +// pointer to a struct that embeds BaseSystemState. +type System interface { + Run() + cardinalSystem() +} + func RegisterSystem[T any](world *World, system func(*T), opts ...SystemOption) { cfg := newSystemConfig() for _, opt := range opts { @@ -40,32 +47,76 @@ func RegisterSystem[T any](world *World, system func(*T), opts ...SystemOption) } name := fmt.Sprintf("%T", system) - fn := func() { system(state) } + registerSystem(world, name, cfg.hook, func() { system(state) }) +} + +// RegisterSystemV2 registers a caller-owned system instance. The instance must +// be a non-nil pointer to a struct that embeds BaseSystemState. +func RegisterSystemV2(world *World, system System, opts ...SystemOption) { + cfg := newSystemConfig() + for _, opt := range opts { + opt(&cfg) + } + + value := reflect.ValueOf(system) + if value.Kind() != reflect.Pointer || value.IsNil() || value.Elem().Kind() != reflect.Struct { + panic(eris.Errorf("system %T must be a non-nil pointer to a struct", system)) + } + + state := value.Elem() + stateType := state.Type() + if _, ok := stateType.MethodByName("Run"); ok { + panic(eris.Errorf("system %T Run method must use a pointer receiver", system)) + } + + baseField, ok := stateType.FieldByName("BaseSystemState") + if !ok || len(baseField.Index) != 1 || !baseField.Anonymous || + baseField.Type != reflect.TypeFor[BaseSystemState]() { + panic(eris.Errorf("system %T must embed cardinal.BaseSystemState", system)) + } - // If debug is enabled, wrap the system function with performance instrumentation. + if err := initSystemV2Fields(state, world); err != nil { + panic(eris.Wrapf(err, "error initializing system fields")) + } + + registerSystem(world, fmt.Sprintf("%T", system), cfg.hook, system.Run) +} + +func registerSystem(world *World, name string, hook SystemHook, run func()) { + fn := run + + // If debug is enabled, wrap the system with performance instrumentation. if world.debug != nil { fn = func() { ts := world.currentTick.timestamp startTime := ts.Add(time.Since(ts)) - system(state) + run() endTime := ts.Add(time.Since(ts)) world.debug.recordSpan(performance.TickSpan{ TickHeight: world.currentTick.height, SystemName: name, - SystemHook: uint8(cfg.hook), + SystemHook: uint8(hook), StartTime: startTime, EndTime: endTime, }) } } - err := ecs.RegisterSystem(world.world, name, cfg.hook, fn) + err := ecs.RegisterSystem(world.world, name, hook, fn) if err != nil { panic(eris.Wrapf(err, "error registering system")) } } func initSystemFields[T any](state *T, world *World) error { + return initSystemFieldValues(reflect.ValueOf(state).Elem(), world, false) +} + +func initSystemV2Fields(state reflect.Value, world *World) error { + return initSystemFieldValues(state, world, true) +} + +func initSystemFieldValues(state reflect.Value, world *World, allowPrivateState bool) error { meta := systemInitMetadata{ world: world, commands: make(map[string]struct{}), @@ -74,20 +125,28 @@ func initSystemFields[T any](state *T, world *World) error { } // For each field in the system state, initialize the field and collect its dependencies. - value := reflect.ValueOf(state).Elem() - for i := range value.NumField() { - field := value.Field(i) - fieldType := value.Type().Field(i) - - // Ignore private implementation state, but keep private Cardinal dependencies - // as fail-fast configuration errors. - if !fieldType.IsExported() { - if field.Addr().Type().Implements(reflect.TypeFor[systemField]()) { + for i := range state.NumField() { + field := state.Field(i) + fieldType := state.Type().Field(i) + + if allowPrivateState && !fieldType.IsExported() { + systemFieldType := reflect.TypeFor[systemField]() + if field.Type().Implements(systemFieldType) || + field.Addr().Type().Implements(systemFieldType) { return eris.Errorf("field %s must be exported", fieldType.Name) } continue } + if allowPrivateState && field.Type().Implements(reflect.TypeFor[systemField]()) { + return eris.Errorf("field %s must be declared as a value", fieldType.Name) + } + + // If the field is not exported, return an error. + if !field.CanAddr() { + return eris.Errorf("field %s must be exported", fieldType.Name) + } + fieldInstance := field.Addr().Interface() cardinalField, ok := fieldInstance.(systemField) @@ -176,6 +235,8 @@ type BaseSystemState struct { world *World } +func (*BaseSystemState) cardinalSystem() {} + func (b *BaseSystemState) init(meta *systemInitMetadata) error { b.world = meta.world return nil diff --git a/pkg/cardinal/system_internal_test.go b/pkg/cardinal/system_internal_test.go index 371137461..afc816d4a 100644 --- a/pkg/cardinal/system_internal_test.go +++ b/pkg/cardinal/system_internal_test.go @@ -23,57 +23,174 @@ type privateStateSystem struct { scratch []int } +func (system *privateStateSystem) Run() { + (*system.dependency)++ + system.scratch = append(system.scratch, *system.dependency) +} + type privateDependencySystem struct { BaseSystemState events WithEvent[testutils.SimpleEvent] } -func TestRegisterSystem_AllowsPersistentPrivateState(t *testing.T) { +func (system *privateDependencySystem) Run() { + _ = system.events +} + +type privatePointerDependencySystem struct { + BaseSystemState + + events *WithEvent[testutils.SimpleEvent] +} + +func (system *privatePointerDependencySystem) Run() { + _ = system.events +} + +type exportedPointerDependencySystem struct { + BaseSystemState + + Events *WithEvent[testutils.SimpleEvent] +} + +func (system *exportedPointerDependencySystem) Run() { + _ = system.Events +} + +type valueSystem struct { + BaseSystemState +} + +func (valueSystem) Run() {} + +type indirectBaseState struct { + BaseSystemState +} + +type indirectBaseSystem struct { + indirectBaseState +} + +func (*indirectBaseSystem) Run() {} + +func TestRegisterSystemV2_UsesCallerOwnedInstance(t *testing.T) { t.Parallel() dependency := 40 world := &World{world: ecs.NewWorld()} - var firstState *privateStateSystem + system := &privateStateSystem{dependency: &dependency} - RegisterSystem(world, func(state *privateStateSystem) { - if state.dependency == nil { - state.dependency = &dependency - } - if firstState == nil { - firstState = state - } - - assert.Same(t, firstState, state) - (*state.dependency)++ - state.scratch = append(state.scratch, *state.dependency) - }) + RegisterSystemV2(world, system) world.world.Init() world.world.Tick() world.world.Tick() - require.NotNil(t, firstState) - assert.Same(t, world, firstState.world) + assert.Same(t, world, system.world) assert.Equal(t, 42, dependency) - assert.Equal(t, []int{41, 42}, firstState.scratch) + assert.Equal(t, []int{41, 42}, system.scratch) } -func TestRegisterSystem_RejectsPrivateCardinalDependency(t *testing.T) { +func TestRegisterSystemV2_HonorsHook(t *testing.T) { t.Parallel() + dependency := 40 world := &World{world: ecs.NewWorld()} + system := &privateStateSystem{dependency: &dependency} + + RegisterSystemV2(world, system, WithHook(Init)) + assert.Equal(t, 40, dependency) + + world.world.Init() + assert.Equal(t, 41, dependency) + + world.world.Tick() + world.world.Tick() + + assert.Equal(t, 41, dependency) + assert.Equal(t, []int{41}, system.scratch) +} + +func TestRegisterSystemV2_RejectsPrivateCardinalDependency(t *testing.T) { + t.Parallel() + + t.Run("value", func(t *testing.T) { + t.Parallel() + + world := &World{world: ecs.NewWorld()} + require.PanicsWithError( + t, + "error initializing system fields: field events must be exported", + func() { + RegisterSystemV2(world, &privateDependencySystem{}) + }, + ) + }) + + t.Run("pointer", func(t *testing.T) { + t.Parallel() + world := &World{world: ecs.NewWorld()} + require.PanicsWithError( + t, + "error initializing system fields: field events must be exported", + func() { + RegisterSystemV2(world, &privatePointerDependencySystem{}) + }, + ) + }) +} + +func TestRegisterSystemV2_RejectsPointerCardinalDependency(t *testing.T) { + t.Parallel() + + world := &World{world: ecs.NewWorld()} require.PanicsWithError( t, - "error initializing system fields: field events must be exported", + "error initializing system fields: field Events must be declared as a value", func() { - RegisterSystem(world, func(state *privateDependencySystem) { - _ = state.events - }) + RegisterSystemV2(world, &exportedPointerDependencySystem{}) }, ) } +func TestRegisterSystemV2_RejectsInvalidInstances(t *testing.T) { + t.Parallel() + + world := &World{world: ecs.NewWorld()} + + t.Run("typed nil", func(t *testing.T) { + t.Parallel() + + var system *privateStateSystem + require.PanicsWithError( + t, + "system *cardinal.privateStateSystem must be a non-nil pointer to a struct", + func() { RegisterSystemV2(world, system) }, + ) + }) + + t.Run("value receiver", func(t *testing.T) { + t.Parallel() + + require.PanicsWithError( + t, + "system *cardinal.valueSystem Run method must use a pointer receiver", + func() { RegisterSystemV2(world, &valueSystem{}) }, + ) + }) + + t.Run("indirect base state", func(t *testing.T) { + t.Parallel() + + require.PanicsWithError( + t, + "system *cardinal.indirectBaseSystem must embed cardinal.BaseSystemState", + func() { RegisterSystemV2(world, &indirectBaseSystem{}) }, + ) + }) +} + // ------------------------------------------------------------------------------------------------- // WithCommand smoke tests // ------------------------------------------------------------------------------------------------- From 6ca1337aecdbfa1b639c9a67844a63cb19a96bf7 Mon Sep 17 00:00:00 2001 From: sms-yui <287818108+sms-yui@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:43:53 +0000 Subject: [PATCH 3/4] style(cardinal): shorten system identifiers --- pkg/cardinal/system.go | 12 +++++----- pkg/cardinal/system_internal_test.go | 36 ++++++++++++++-------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/cardinal/system.go b/pkg/cardinal/system.go index bc3a60140..26294c783 100644 --- a/pkg/cardinal/system.go +++ b/pkg/cardinal/system.go @@ -52,34 +52,34 @@ func RegisterSystem[T any](world *World, system func(*T), opts ...SystemOption) // RegisterSystemV2 registers a caller-owned system instance. The instance must // be a non-nil pointer to a struct that embeds BaseSystemState. -func RegisterSystemV2(world *World, system System, opts ...SystemOption) { +func RegisterSystemV2(world *World, s System, opts ...SystemOption) { cfg := newSystemConfig() for _, opt := range opts { opt(&cfg) } - value := reflect.ValueOf(system) + value := reflect.ValueOf(s) if value.Kind() != reflect.Pointer || value.IsNil() || value.Elem().Kind() != reflect.Struct { - panic(eris.Errorf("system %T must be a non-nil pointer to a struct", system)) + panic(eris.Errorf("system %T must be a non-nil pointer to a struct", s)) } state := value.Elem() stateType := state.Type() if _, ok := stateType.MethodByName("Run"); ok { - panic(eris.Errorf("system %T Run method must use a pointer receiver", system)) + panic(eris.Errorf("system %T Run method must use a pointer receiver", s)) } baseField, ok := stateType.FieldByName("BaseSystemState") if !ok || len(baseField.Index) != 1 || !baseField.Anonymous || baseField.Type != reflect.TypeFor[BaseSystemState]() { - panic(eris.Errorf("system %T must embed cardinal.BaseSystemState", system)) + panic(eris.Errorf("system %T must embed cardinal.BaseSystemState", s)) } if err := initSystemV2Fields(state, world); err != nil { panic(eris.Wrapf(err, "error initializing system fields")) } - registerSystem(world, fmt.Sprintf("%T", system), cfg.hook, system.Run) + registerSystem(world, fmt.Sprintf("%T", s), cfg.hook, s.Run) } func registerSystem(world *World, name string, hook SystemHook, run func()) { diff --git a/pkg/cardinal/system_internal_test.go b/pkg/cardinal/system_internal_test.go index afc816d4a..1632be733 100644 --- a/pkg/cardinal/system_internal_test.go +++ b/pkg/cardinal/system_internal_test.go @@ -23,9 +23,9 @@ type privateStateSystem struct { scratch []int } -func (system *privateStateSystem) Run() { - (*system.dependency)++ - system.scratch = append(system.scratch, *system.dependency) +func (s *privateStateSystem) Run() { + (*s.dependency)++ + s.scratch = append(s.scratch, *s.dependency) } type privateDependencySystem struct { @@ -34,8 +34,8 @@ type privateDependencySystem struct { events WithEvent[testutils.SimpleEvent] } -func (system *privateDependencySystem) Run() { - _ = system.events +func (s *privateDependencySystem) Run() { + _ = s.events } type privatePointerDependencySystem struct { @@ -44,8 +44,8 @@ type privatePointerDependencySystem struct { events *WithEvent[testutils.SimpleEvent] } -func (system *privatePointerDependencySystem) Run() { - _ = system.events +func (s *privatePointerDependencySystem) Run() { + _ = s.events } type exportedPointerDependencySystem struct { @@ -54,8 +54,8 @@ type exportedPointerDependencySystem struct { Events *WithEvent[testutils.SimpleEvent] } -func (system *exportedPointerDependencySystem) Run() { - _ = system.Events +func (s *exportedPointerDependencySystem) Run() { + _ = s.Events } type valueSystem struct { @@ -79,16 +79,16 @@ func TestRegisterSystemV2_UsesCallerOwnedInstance(t *testing.T) { dependency := 40 world := &World{world: ecs.NewWorld()} - system := &privateStateSystem{dependency: &dependency} + s := &privateStateSystem{dependency: &dependency} - RegisterSystemV2(world, system) + RegisterSystemV2(world, s) world.world.Init() world.world.Tick() world.world.Tick() - assert.Same(t, world, system.world) + assert.Same(t, world, s.world) assert.Equal(t, 42, dependency) - assert.Equal(t, []int{41, 42}, system.scratch) + assert.Equal(t, []int{41, 42}, s.scratch) } func TestRegisterSystemV2_HonorsHook(t *testing.T) { @@ -96,9 +96,9 @@ func TestRegisterSystemV2_HonorsHook(t *testing.T) { dependency := 40 world := &World{world: ecs.NewWorld()} - system := &privateStateSystem{dependency: &dependency} + s := &privateStateSystem{dependency: &dependency} - RegisterSystemV2(world, system, WithHook(Init)) + RegisterSystemV2(world, s, WithHook(Init)) assert.Equal(t, 40, dependency) world.world.Init() @@ -108,7 +108,7 @@ func TestRegisterSystemV2_HonorsHook(t *testing.T) { world.world.Tick() assert.Equal(t, 41, dependency) - assert.Equal(t, []int{41}, system.scratch) + assert.Equal(t, []int{41}, s.scratch) } func TestRegisterSystemV2_RejectsPrivateCardinalDependency(t *testing.T) { @@ -162,11 +162,11 @@ func TestRegisterSystemV2_RejectsInvalidInstances(t *testing.T) { t.Run("typed nil", func(t *testing.T) { t.Parallel() - var system *privateStateSystem + var s *privateStateSystem require.PanicsWithError( t, "system *cardinal.privateStateSystem must be a non-nil pointer to a struct", - func() { RegisterSystemV2(world, system) }, + func() { RegisterSystemV2(world, s) }, ) }) From 6db6cca307f9a87c553109af152788fcfed7e4ce Mon Sep 17 00:00:00 2001 From: Scott Date: Wed, 29 Jul 2026 12:45:15 -0700 Subject: [PATCH 4/4] refactor(cardinal): make struct registration generic --- pkg/cardinal/system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cardinal/system.go b/pkg/cardinal/system.go index 26294c783..29122282f 100644 --- a/pkg/cardinal/system.go +++ b/pkg/cardinal/system.go @@ -52,7 +52,7 @@ func RegisterSystem[T any](world *World, system func(*T), opts ...SystemOption) // RegisterSystemV2 registers a caller-owned system instance. The instance must // be a non-nil pointer to a struct that embeds BaseSystemState. -func RegisterSystemV2(world *World, s System, opts ...SystemOption) { +func RegisterSystemV2[S System](world *World, s S, opts ...SystemOption) { cfg := newSystemConfig() for _, opt := range opts { opt(&cfg)