Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(cardinal): log a warning if RegisterSystems encounters a duplicate System name #411

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cardinal/cardinal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestCanQueryInsideSystem(t *testing.T) {
_, err := cardinal.CreateMany(wCtx, wantNumOfEntities, Foo{})
assert.NilError(t, err)
gotNumOfEntities := 0
cardinal.RegisterSystems(world, func(worldCtx cardinal.WorldContext) error {
err = cardinal.RegisterSystems(world, func(worldCtx cardinal.WorldContext) error {
q, err := worldCtx.NewSearch(cardinal.Exact(Foo{}))
assert.NilError(t, err)
err = q.Each(worldCtx, func(cardinal.EntityID) bool {
Expand All @@ -46,6 +46,7 @@ func TestCanQueryInsideSystem(t *testing.T) {
assert.NilError(t, err)
return nil
})
assert.NilError(t, err)

doTick()
assert.Equal(t, world.CurrentTick(), uint64(1))
Expand Down
2 changes: 1 addition & 1 deletion cardinal/ecs/benchmark/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func setupWorld(t testing.TB, numOfEntities int, enableHealthSystem bool) *ecs.W
world := newWorldWithRealRedis(t)
zerolog.SetGlobalLevel(zerolog.Disabled)
if enableHealthSystem {
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
q, err := world.NewSearch(ecs.Contains(Health{}))
assert.NilError(t, err)
err = q.Each(wCtx, func(id entity.ID) bool {
Expand Down
2 changes: 1 addition & 1 deletion cardinal/ecs/chain_recover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestWorld_RecoverFromChain(t *testing.T) {
sysRuns := uint64(0)
timesSendEnergyRan := 0
// send energy system
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
sysRuns++
txs := sendEnergyTx.In(wCtx)
if len(txs) > 0 {
Expand Down
2 changes: 1 addition & 1 deletion cardinal/ecs/ecs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestECS(t *testing.T) {
_, err = component.CreateMany(wCtx, numEnergyOnly, EnergyComponent{})
assert.NilError(t, err)

world.AddSystem(UpdateEnergySystem)
world.RegisterSystem(UpdateEnergySystem)
assert.NilError(t, world.LoadGameState())

assert.NilError(t, world.Tick(context.Background()))
Expand Down
17 changes: 16 additions & 1 deletion cardinal/ecs/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@ func testSystemWarningTrigger(wCtx ecs.WorldContext) error {
return testSystem(wCtx)
}

func TestWarningLogIfDuplicateSystemRegistered(t *testing.T) {
w := ecs.NewTestWorld(t)
// replaces internal Logger with one that logs to the buf variable above.
var buf bytes.Buffer
bufLogger := zerolog.New(&buf)
cardinalLogger := log.Logger{
Logger: &bufLogger,
}
w.InjectLogger(&cardinalLogger)
sysName := "foo"
w.RegisterSystemWithName(testSystem, sysName)
w.RegisterSystemWithName(testSystem, sysName)
assert.Check(t, strings.Contains(buf.String(), "duplicate system registered: "+sysName))
}

func TestWorldLogger(t *testing.T) {
w := ecs.NewTestWorld(t)
// replaces internal Logger with one that logs to the buf variable above.
Expand Down Expand Up @@ -142,7 +157,7 @@ func TestWorldLogger(t *testing.T) {

// create a system for logging.
buf.Reset()
w.AddSystems(testSystemWarningTrigger)
w.RegisterSystems(testSystemWarningTrigger)
err = w.LoadGameState()
assert.NilError(t, err)
ctx := context.Background()
Expand Down
24 changes: 12 additions & 12 deletions cardinal/ecs/message/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestCanQueueTransactions(t *testing.T) {
assert.NilError(t, err)

// Set up a system that allows for the modification of a player's score
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
modifyScore := modifyScoreMsg.In(wCtx)
for _, txData := range modifyScore {
ms := txData.Msg
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestSystemsAreExecutedDuringGameTick(t *testing.T) {
wCtx := ecs.NewWorldContext(world)
id, err := component.Create(wCtx, CounterComponent{})
assert.NilError(t, err)
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
return component.UpdateComponent[CounterComponent](wCtx, id, func(c *CounterComponent) *CounterComponent {
c.Count++
return c
Expand All @@ -144,7 +144,7 @@ func TestTransactionAreAppliedToSomeEntities(t *testing.T) {
modifyScoreMsg := ecs.NewMessageType[*ModifyScoreMsg, *EmptyMsgResult]("modify_score")
assert.NilError(t, world.RegisterMessages(modifyScoreMsg))

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
modifyScores := modifyScoreMsg.In(wCtx)
for _, msData := range modifyScores {
ms := msData.Msg
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestAddToQueueDuringTickDoesNotTimeout(t *testing.T) {
inSystemCh := make(chan struct{})
// This system will block forever. This will give us a never-ending game tick that we can use
// to verify that the addition of more transactions doesn't block.
world.AddSystem(func(ecs.WorldContext) error {
world.RegisterSystem(func(ecs.WorldContext) error {
<-inSystemCh
select {}
})
Expand Down Expand Up @@ -252,13 +252,13 @@ func TestTransactionsAreExecutedAtNextTick(t *testing.T) {
// transaction queue. These counts should be the same for each tick. modScoreCountCh is an unbuffered channel
// so these systems will block while writing to modScoreCountCh. This allows the test to ensure that we can run
// commands mid-tick.
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
modScores := modScoreMsg.In(wCtx)
modScoreCountCh <- len(modScores)
return nil
})

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
modScores := modScoreMsg.In(wCtx)
modScoreCountCh <- len(modScores)
return nil
Expand Down Expand Up @@ -321,7 +321,7 @@ func TestIdenticallyTypedTransactionCanBeDistinguished(t *testing.T) {
alpha.AddToQueue(world, NewOwner{"alpha"})
beta.AddToQueue(world, NewOwner{"beta"})

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
newNames := alpha.In(wCtx)
assert.Check(t, len(newNames) == 1, "expected 1 transaction, not %d", len(newNames))
assert.Check(t, newNames[0].Msg.Name == "alpha")
Expand Down Expand Up @@ -409,7 +409,7 @@ func TestCanGetTransactionErrorsAndResults(t *testing.T) {
wantSecondError := errors.New("another transaction error")
wantDeltaX, wantDeltaY := 99, 100

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
// This new In function returns a triplet of information:
// 1) The transaction input
// 2) An ID that uniquely identifies this specific transaction
Expand Down Expand Up @@ -463,7 +463,7 @@ func TestSystemCanFindErrorsFromEarlierSystem(t *testing.T) {
assert.NilError(t, world.RegisterMessages(numTx))
wantErr := errors.New("some transaction error")
systemCalls := 0
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
systemCalls++
txs := numTx.In(wCtx)
assert.Equal(t, 1, len(txs))
Expand All @@ -474,7 +474,7 @@ func TestSystemCanFindErrorsFromEarlierSystem(t *testing.T) {
return nil
})

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
systemCalls++
txs := numTx.In(wCtx)
assert.Equal(t, 1, len(txs))
Expand Down Expand Up @@ -507,7 +507,7 @@ func TestSystemCanClobberTransactionResult(t *testing.T) {

firstResult := MsgOut{1234}
secondResult := MsgOut{5678}
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
systemCalls++
txs := numTx.In(wCtx)
assert.Equal(t, 1, len(txs))
Expand All @@ -518,7 +518,7 @@ func TestSystemCanClobberTransactionResult(t *testing.T) {
return nil
})

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
systemCalls++
txs := numTx.In(wCtx)
assert.Equal(t, 1, len(txs))
Expand Down
2 changes: 1 addition & 1 deletion cardinal/ecs/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestForEachTransaction(t *testing.T) {
someMsg := ecs.NewMessageType[SomeMsgRequest, SomeMsgResponse]("some_msg")
assert.NilError(t, world.RegisterMessages(someMsg))

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
someMsg.ForEach(wCtx, func(t ecs.TxData[SomeMsgRequest]) (result SomeMsgResponse, err error) {
if t.Msg.GenerateError {
return result, errors.New("some error")
Expand Down
4 changes: 2 additions & 2 deletions cardinal/ecs/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func TestCanReloadState(t *testing.T) {
assert.NilError(t, err)
oneAlphaNum, err := alphaWorld.GetComponentByName(oneAlphaNumComp{}.Name())
assert.NilError(t, err)
alphaWorld.AddSystem(func(wCtx ecs.WorldContext) error {
alphaWorld.RegisterSystem(func(wCtx ecs.WorldContext) error {
q, err := wCtx.NewSearch(ecs.Contains(oneAlphaNum))
if err != nil {
return err
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestCanFindTransactionsAfterReloadingWorld(t *testing.T) {
for reload := 0; reload < 5; reload++ {
world := testutil.InitWorldWithRedis(t, redisStore)
assert.NilError(t, world.RegisterMessages(someTx))
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
for _, tx := range someTx.In(wCtx) {
someTx.SetResult(wCtx, tx.Hash, Result{})
}
Expand Down
10 changes: 5 additions & 5 deletions cardinal/ecs/tick_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestIfPanicMessageLogged(t *testing.T) {
w.InjectLogger(&cardinalLogger)
// In this test, our "buggy" system fails once Power reaches 3
errorTxt := "BIG ERROR OH NO"
w.AddSystem(func(ecs.WorldContext) error {
w.RegisterSystem(func(ecs.WorldContext) error {
panic(errorTxt)
})
assert.NilError(t, w.LoadGameState())
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestCanIdentifyAndFixSystemError(t *testing.T) {
errorSystem := errors.New("3 power? That's too much, man")

// In this test, our "buggy" system fails once Power reaches 3
oneWorld.AddSystem(func(wCtx ecs.WorldContext) error {
oneWorld.RegisterSystem(func(wCtx ecs.WorldContext) error {
p, err := component.GetComponent[onePowerComponent](wCtx, id)
if err != nil {
return err
Expand All @@ -146,7 +146,7 @@ func TestCanIdentifyAndFixSystemError(t *testing.T) {
assert.NilError(t, ecs.RegisterComponent[twoPowerComponent](twoWorld))

// this is our fixed system that can handle Power levels of 3 and higher
twoWorld.AddSystem(func(wCtx ecs.WorldContext) error {
twoWorld.RegisterSystem(func(wCtx ecs.WorldContext) error {
p, err := component.GetComponent[onePowerComponent](wCtx, id)
if err != nil {
return err
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestCanRecoverStateAfterFailedArchetypeChange(t *testing.T) {
}

errorToggleComponent := errors.New("problem with toggle component")
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
// Get the one and only entity ID
q, err := wCtx.NewSearch(ecs.Contains(ScalarComponentStatic{}))
assert.NilError(t, err)
Expand Down Expand Up @@ -327,7 +327,7 @@ func TestCanRecoverTransactionsFromFailedSystemRun(t *testing.T) {
powerTx := ecs.NewMessageType[PowerComp, PowerComp]("change_power")
assert.NilError(t, world.RegisterMessages(powerTx))

world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
q, err := wCtx.NewSearch(ecs.Contains(PowerComp{}))
assert.NilError(t, err)
id := q.MustFirst(wCtx)
Expand Down
25 changes: 19 additions & 6 deletions cardinal/ecs/world.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,17 @@ func (w *World) GetTxQueueAmount() int {
return w.txQueue.GetAmountOfTxs()
}

func (w *World) AddSystem(s System) {
w.AddSystemWithName(s, "")
func (w *World) RegisterSystem(s System) {
w.RegisterSystemWithName(s, "")
}

func (w *World) AddSystems(systems ...System) {
func (w *World) RegisterSystems(systems ...System) {
for _, system := range systems {
w.AddSystemWithName(system, "")
w.RegisterSystemWithName(system, "")
}
}

func (w *World) AddSystemWithName(system System, functionName string) {
func (w *World) RegisterSystemWithName(system System, functionName string) {
if w.stateIsLoaded {
panic("cannot register systems after loading game state")
}
Expand All @@ -146,6 +146,19 @@ func (w *World) AddSystemWithName(system System, functionName string) {
w.systemNames = append(w.systemNames, functionName)
// appends registeredSystem into the member system list in world.
w.systems = append(w.systems, system)
w.checkDuplicateSystemName()
}

func (w *World) checkDuplicateSystemName() {
mappedNames := make(map[string]int, len(w.systemNames))
for _, sysName := range w.systemNames {
if sysName != "" {
mappedNames[sysName]++
if mappedNames[sysName] > 1 {
w.Logger.Warn().Msgf("duplicate system registered: %s", sysName)
}
}
}
}

func (w *World) AddInitSystem(system System) {
Expand Down Expand Up @@ -275,7 +288,7 @@ func NewWorld(nonceStore storage.NonceStorage, entityStore store.IManager, opts
addChannelWaitingForNextTick: make(chan chan struct{}),
}
w.isGameLoopRunning.Store(false)
w.AddSystems(RegisterPersonaSystem, AuthorizePersonaAddressSystem)
w.RegisterSystems(RegisterPersonaSystem, AuthorizePersonaAddressSystem)
err := RegisterComponent[SignerComponent](w)
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions cardinal/ecs/world_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestEVMTxConsume(t *testing.T) {
assert.NilError(t, w.RegisterMessages(fooTx))
var returnVal FooOut
var returnErr error
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
fooTx.ForEach(wCtx, func(t ecs.TxData[FooIn]) (FooOut, error) {
return returnVal, returnErr
})
Expand Down Expand Up @@ -167,7 +167,7 @@ func TestAddSystems(t *testing.T) {
}

w := ecs.NewTestWorld(t)
w.AddSystems(sys, sys, sys)
w.RegisterSystems(sys, sys, sys)
err := w.LoadGameState()
assert.NilError(t, err)

Expand All @@ -180,7 +180,7 @@ func TestAddSystems(t *testing.T) {
func TestSystemExecutionOrder(t *testing.T) {
w := ecs.NewTestWorld(t)
order := make([]int, 0, 3)
w.AddSystems(func(ecs.WorldContext) error {
w.RegisterSystems(func(ecs.WorldContext) error {
order = append(order, 1)
return nil
}, func(ecs.WorldContext) error {
Expand Down
4 changes: 2 additions & 2 deletions cardinal/events/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestEventsThroughSystems(t *testing.T) {
counter1 := atomic.Int32{}
counter1.Store(0)
for i := 0; i < numberToTest; i++ {
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
wCtx.GetWorld().EmitEvent(&events.Event{Message: "test"})
counter1.Add(1)
return nil
Expand Down Expand Up @@ -157,7 +157,7 @@ func TestEventHubLogger(t *testing.T) {
w := ecs.NewTestWorld(t, ecs.WithLoggingEventHub(&cardinalLogger))
numberToTest := 5
for i := 0; i < numberToTest; i++ {
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
wCtx.GetWorld().EmitEvent(&events.Event{Message: "test"})
return nil
})
Expand Down
2 changes: 1 addition & 1 deletion cardinal/evm/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestServer_SendMessage(t *testing.T) {
currBarIndex := 0

// add a system that checks that they are submitted properly to the world in the correct order.
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
inFooTxs := fooTx.In(wCtx)
inBarTxs := barTx.In(wCtx)
if len(inFooTxs) == 0 && len(inBarTxs) == 0 {
Expand Down
5 changes: 4 additions & 1 deletion cardinal/example_messagetype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func ExampleMessageType() {
panic(err)
}

cardinal.RegisterSystems(world, func(wCtx cardinal.WorldContext) error {
err = cardinal.RegisterSystems(world, func(wCtx cardinal.WorldContext) error {
for _, tx := range MoveMsg.In(wCtx) {
msg := tx.Msg()
// handle the msg
Expand All @@ -47,6 +47,9 @@ func ExampleMessageType() {
}
return nil
})
if err != nil {
panic(err)
}
// The above system will be called during each game tick.

err = world.StartGame()
Expand Down
3 changes: 2 additions & 1 deletion cardinal/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestTransactionExample(t *testing.T) {
world, doTick := testutils.MakeWorldAndTicker(t)
assert.NilError(t, cardinal.RegisterComponent[Health](world))
assert.NilError(t, cardinal.RegisterMessages(world, addHealthToEntity))
cardinal.RegisterSystems(world, func(worldCtx cardinal.WorldContext) error {
err := cardinal.RegisterSystems(world, func(worldCtx cardinal.WorldContext) error {
// test "In" method
for _, tx := range addHealthToEntity.In(worldCtx) {
targetID := tx.Msg().TargetID
Expand Down Expand Up @@ -67,6 +67,7 @@ func TestTransactionExample(t *testing.T) {

return nil
})
assert.NilError(t, err)

testWorldCtx := testutils.WorldToWorldContext(world)
ids, err := cardinal.CreateMany(testWorldCtx, 10, Health{})
Expand Down
Loading