diff --git a/pkg/cardinal/system.go b/pkg/cardinal/system.go index e5b3f5b0d..29122282f 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[S System](world *World, s S, opts ...SystemOption) { + cfg := newSystemConfig() + for _, opt := range opts { + opt(&cfg) + } - // If debug is enabled, wrap the system function with performance instrumentation. + 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", 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", 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", s)) + } + + if err := initSystemV2Fields(state, world); err != nil { + panic(eris.Wrapf(err, "error initializing system fields")) + } + + registerSystem(world, fmt.Sprintf("%T", s), cfg.hook, s.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,10 +125,22 @@ 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) + 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() { @@ -172,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 17f4532b1..1632be733 100644 --- a/pkg/cardinal/system_internal_test.go +++ b/pkg/cardinal/system_internal_test.go @@ -16,6 +16,181 @@ import ( // TODO: test system registration, e.g. duplicate field detection, etc. +type privateStateSystem struct { + BaseSystemState + + dependency *int + scratch []int +} + +func (s *privateStateSystem) Run() { + (*s.dependency)++ + s.scratch = append(s.scratch, *s.dependency) +} + +type privateDependencySystem struct { + BaseSystemState + + events WithEvent[testutils.SimpleEvent] +} + +func (s *privateDependencySystem) Run() { + _ = s.events +} + +type privatePointerDependencySystem struct { + BaseSystemState + + events *WithEvent[testutils.SimpleEvent] +} + +func (s *privatePointerDependencySystem) Run() { + _ = s.events +} + +type exportedPointerDependencySystem struct { + BaseSystemState + + Events *WithEvent[testutils.SimpleEvent] +} + +func (s *exportedPointerDependencySystem) Run() { + _ = s.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()} + s := &privateStateSystem{dependency: &dependency} + + RegisterSystemV2(world, s) + world.world.Init() + world.world.Tick() + world.world.Tick() + + assert.Same(t, world, s.world) + assert.Equal(t, 42, dependency) + assert.Equal(t, []int{41, 42}, s.scratch) +} + +func TestRegisterSystemV2_HonorsHook(t *testing.T) { + t.Parallel() + + dependency := 40 + world := &World{world: ecs.NewWorld()} + s := &privateStateSystem{dependency: &dependency} + + RegisterSystemV2(world, s, 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}, s.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 declared as a value", + func() { + 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 s *privateStateSystem + require.PanicsWithError( + t, + "system *cardinal.privateStateSystem must be a non-nil pointer to a struct", + func() { RegisterSystemV2(world, s) }, + ) + }) + + 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 // -------------------------------------------------------------------------------------------------