chore(cardinal): log a warning if RegisterSystems encounters a duplic…

…ate System name (#411)

cardinal/cardinal_test.go

@@ -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 {
@@ -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))

cardinal/ecs/benchmark/benchmark_test.go

@@ -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 {

cardinal/ecs/chain_recover_test.go

@@ -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 {

cardinal/ecs/ecs_test.go

@@ -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()))

cardinal/ecs/log/log_test.go

@@ -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.
@@ -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()

cardinal/ecs/message/message_test.go

@@ -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
@@ -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
@@ -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
@@ -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 {}
 	})
@@ -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
@@ -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")
@@ -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
@@ -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))
@@ -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))
@@ -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))
@@ -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))

cardinal/ecs/message_test.go

@@ -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")

cardinal/ecs/state_test.go

@@ -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
@@ -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{})
 			}

cardinal/ecs/tick_test.go

@@ -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())
@@ -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
@@ -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
@@ -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)
@@ -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)

cardinal/ecs/world.go

@@ -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")
 	}
@@ -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) {
@@ -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

cardinal/ecs/world_test.go

@@ -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
 		})
@@ -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)
 
@@ -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 {

cardinal/events/events_test.go

@@ -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
@@ -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
 		})

cardinal/evm/server_test.go

@@ -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 {

cardinal/example_messagetype_test.go

@@ -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
@@ -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()

cardinal/message_test.go

@@ -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
@@ -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{})