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 1 commit
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
2 changes: 1 addition & 1 deletion cardinal/ecs/benchmark/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func setupWorld(t testing.TB, numOfEntities int, enableHealthSystem bool) *ecs.W
disabledLogger := world.Logger.Level(zerolog.Disabled)
world.InjectLogger(&ecslog.Logger{&disabledLogger})
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
2 changes: 1 addition & 1 deletion cardinal/ecs/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,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
30 changes: 24 additions & 6 deletions cardinal/ecs/world.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,17 @@
return w.txQueue.GetAmountOfTxs()
}

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

Check failure on line 125 in cardinal/ecs/world.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Error return value of `w.RegisterSystemWithName` is not checked (errcheck)
}

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

Check failure on line 130 in cardinal/ecs/world.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Error return value of `w.RegisterSystemWithName` is not checked (errcheck)
}
}

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

func (w *World) checkDuplicateSystemName() error {
mappedNames := make(map[string]struct{}, len(w.systemNames))
for _, sysName := range w.systemNames {
if sysName != "" {
mappedNames[sysName] = struct{}{}
}
}
for _, sysName := range w.systemNames {
if sysName != "" {
if _, ok := mappedNames[sysName]; ok {
return fmt.Errorf("found duplicate system registered: %s", sysName)
}
}
}
return nil
}

func (w *World) AddInitSystem(system System) {
Expand Down Expand Up @@ -270,7 +288,7 @@
evmTxReceipts: make(map[string]EVMTxReceipt),
}
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 @@ -42,7 +42,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 @@ -91,7 +91,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 @@ -104,7 +104,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) {
enabled := false

// add a system that checks that they are submitted properly to the world.
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
if !enabled {
return nil
}
Expand Down
14 changes: 7 additions & 7 deletions cardinal/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestShutDownViaSignal(t *testing.T) {
w := ecs.NewTestWorld(t)
sendTx := ecs.NewMessageType[SendEnergyTx, SendEnergyTxResult]("sendTx")
assert.NilError(t, w.RegisterMessages(sendTx))
w.AddSystem(func(ecs.WorldContext) error {
w.RegisterSystem(func(ecs.WorldContext) error {
return nil
})
assert.NilError(t, w.LoadGameState())
Expand Down Expand Up @@ -247,7 +247,7 @@ func TestHandleTransactionWithNoSignatureVerification(t *testing.T) {
sendTx := ecs.NewMessageType[SendEnergyTx, SendEnergyTxResult](endpoint)
assert.NilError(t, w.RegisterMessages(sendTx))
count := 0
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
txs := sendTx.In(wCtx)
assert.Equal(t, 1, len(txs))
tx := txs[0]
Expand Down Expand Up @@ -304,7 +304,7 @@ func TestHandleSwaggerServer(t *testing.T) {
w := ecs.NewTestWorld(t)
sendTx := ecs.NewMessageType[SendEnergyTx, SendEnergyTxResult]("send-energy")
assert.NilError(t, w.RegisterMessages(sendTx))
w.AddSystem(func(ecs.WorldContext) error {
w.RegisterSystem(func(ecs.WorldContext) error {
return nil
})

Expand Down Expand Up @@ -497,7 +497,7 @@ func TestHandleWrappedTransactionWithNoSignatureVerification(t *testing.T) {
w := ecs.NewTestWorld(t)
sendTx := ecs.NewMessageType[SendEnergyTx, SendEnergyTxResult](endpoint)
assert.NilError(t, w.RegisterMessages(sendTx))
w.AddSystem(func(wCtx ecs.WorldContext) error {
w.RegisterSystem(func(wCtx ecs.WorldContext) error {
txs := sendTx.In(wCtx)
assert.Equal(t, 1, len(txs))
tx := txs[0]
Expand Down Expand Up @@ -908,7 +908,7 @@ func TestCanGetTransactionReceiptsSwagger(t *testing.T) {
world := ecs.NewTestWorld(t)
assert.NilError(t, world.RegisterMessages(incTx, dupeTx, errTx))
// System to handle incrementing numbers
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
for _, tx := range incTx.In(wCtx) {
incTx.SetResult(wCtx, tx.Hash, IncReply{
Number: tx.Msg.Number + 1,
Expand All @@ -917,7 +917,7 @@ func TestCanGetTransactionReceiptsSwagger(t *testing.T) {
return nil
})
// System to handle duplicating strings
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
for _, tx := range dupeTx.In(wCtx) {
dupeTx.SetResult(wCtx, tx.Hash, DupeReply{
Str: tx.Msg.Str + tx.Msg.Str,
Expand All @@ -927,7 +927,7 @@ func TestCanGetTransactionReceiptsSwagger(t *testing.T) {
})
wantError := errors.New("some error")
// System to handle error production
world.AddSystem(func(wCtx ecs.WorldContext) error {
world.RegisterSystem(func(wCtx ecs.WorldContext) error {
for _, tx := range errTx.In(wCtx) {
errTx.AddError(wCtx, tx.Hash, wantError)
errTx.AddError(wCtx, tx.Hash, wantError)
Expand Down
Loading
Loading