DEV-1096 - add a new "pipeline" concept #67
Conversation
- added new Pipeline type which is a series of stages - added a global registry to facilitate plugin architecture - 100% test coverage
Codecov Report
@@ Coverage Diff @@
## main #67 +/- ##
==========================================
+ Coverage 65.67% 67.78% +2.10%
==========================================
Files 12 13 +1
Lines 979 984 +5
==========================================
+ Hits 643 667 +24
+ Misses 316 309 -7
+ Partials 20 8 -12
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
IsGameOver needs to run pipeline, move snakes needs to no-op for that
Support error checking before calling Execute()
torbensky
requested review from
robbles,
bvanvugt and
chris-bsnake
as code owners
pipeline.go
Outdated
| for _, s := range stageNames { | ||
| fn, ok := registry[s] | ||
| if !ok { | ||
| return pipeline{err: errors.New("stage not found")} |
There was a problem hiding this comment.
It'll be handy for troubleshooting if this error includes the unknown stage name. Ran into this already during testing when I misspelled one and couldn't see it right away 😅 .
There was a problem hiding this comment.
Also, if you can make these errors a specific type, e.g. rules.RulesetError or a new type specific to Pipelines, then engine can detect these errors and return HTTP 400 and a better message for bad game creation requests.
It's not consistent, but we do this in a few places already:
https://github.com/BattlesnakeOfficial/rules/blob/main/board.go#L101
There was a problem hiding this comment.
Realized just now that engine could definitely do this internally, since it knows when it's getting the error via rules, and the assumption is that any rules error on game creation is an invalid request from the client.
It's probably more consistent if rules returns a specific error type though?
There was a problem hiding this comment.
An error type is a good idea. I had originally setup the error message to include the stage name, but then I removed it because I wanted the error to be static for simpler comparison. But it did nag at me that it would be useful to know which stage caused the error. I think an error type would allow the best of both worlds - a convenient way to check for the error type and a clear message.
| StageFeedSnakesStandard, | ||
| StageSpawnFoodStandard, | ||
| StageEliminationStandard, | ||
| StageSpawnFoodNoFood, |
There was a problem hiding this comment.
Just noticed this - We have both spawn_food.standard and spawn_food.no_food stages - technically how it worked before, but probably redundant now?
Now I'm wondering if constrictor could just be implemented as this list without either stage?
There was a problem hiding this comment.
haha yeah - that is a bit silly!
There was a problem hiding this comment.
oh, I left spawn_food.no_food, but I'm seeing now that you mentioned we should remove it
There was a problem hiding this comment.
I'll need to update some tests to handle that because there are a few that create initial board states with food and expect the ruleset to remove that
There was a problem hiding this comment.
... although part of me feels safer with it still in there, just in case...
This PR adds a new concept called "pipelines" to the rules package. It is intended as a replacement for the "ruleset" concept. While Rulesets are still supported, the internal implementation of them has been re-written to be entirely Pipeline based, so there is no significant reason to continue using them in new code.
A "pipeline" is a series of "stage functions" which are executed in sequence, one after another, in order to modify a game state and produce a next game state. Pipelines can also be executed to initialise a game state and to check if a game has ended. The design of the pipeline system is intended to facilitate easy customization and extension of game modes. Each existing game mode has been broken down into a series of bite-sized logic, called "stages", so that the stages can be re-used in other game modes. For example, the logic to move snakes each turn is its own stage. Any other game type that wishes to have this type of movement behavior can simply include this stage.
Overview: