Skip to content

Commit

Permalink
disallow registering a named system twice
Browse files Browse the repository at this point in the history
  • Loading branch information
technicallyty committed Nov 9, 2023
1 parent dee26dd commit e48da8e
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 47 deletions.
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 @@ 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, "")

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 @@ 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)
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 @@ func NewWorld(nonceStore storage.NonceStorage, entityStore store.IManager, opts
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

0 comments on commit e48da8e

Please sign in to comment.