Split maze Subtypes into Subtypes and ResultsHandlers #20882
Conversation
Hamms
force-pushed
the
subtype-results
branch
2 times, most recently
from
000f7f8
to
828a2be
Compare
Hamms
force-pushed
the
maze-controller
branch
from
6a0f940
to
b72f871
Compare
Hamms
force-pushed
the
subtype-results
branch
from
b9e7e61
to
78cacb2
Compare
Hamms
changed the title
There was a problem hiding this comment.
I'd like to understand a little better what the extraction plan is for maze/results/*.
If we do extract it, there are some trailing references to TestResults, StudioApp, etc. If we don't, we're reaching into a lot of the Maze internal state. I'm sure you already have a plan for this, just trying to get up to speed 😄.
apps/src/maze/bee.js
Outdated
| // at each location, tracks whether user checked to see if it was a flower or | ||
| // honeycomb using an if block | ||
| this.userChecks_ = []; | ||
|
|
||
| this.overrideStepSpeed = 2; | ||
| this.honey_; | ||
| this.nectars_; |
There was a problem hiding this comment.
Same feedback: let's init these to undefined otherwise it's a no-op.
apps/src/maze/maze.js
Outdated
| @@ -40,6 +42,7 @@ module.exports = class Maze { | |||
|
|
|||
| this.cachedBlockStates = []; | |||
|
|
|||
| this.resultsHandler; | |||
| this.response; | |||
| this.result; | |||
| this.testResults; | |||
apps/src/maze/results/utils.js
Outdated
| const PlanterHandler = require('./planter'); | ||
| const ResultsHandler = require('./resultsHandler'); | ||
|
|
||
| export function createResultsHandlerForSubtype(controller, config) { |
There was a problem hiding this comment.
I'm not a huge fan of this helper but I can't quite put my finger on why. Can we use a hash of the subtype name to the appropriate handler, and look up by controller.subtype.constructor.name?
Will the extracted repo include maze/results/*?
There was a problem hiding this comment.
Sure!
And (as answered in another comment) no it will not
apps/src/maze/results/bee.js
Outdated
| BeeTerminationValue as TerminationValue | ||
| } from '../../constants.js'; | ||
|
|
||
| export default class Bee extends Gatherer { |
There was a problem hiding this comment.
Let's export BeeHandler here, to match how it's used in maze/results/utils.js.
|
The idea is that The idea is that results is looking into a lot of maze state, because that's where we have the concept of success or failure as it relates to maze state. But it should not in any way be modifying maze state |
|
PTAL |
… care at all about 'success'
…edback out into standalone ResultsHandler
… update the results handler factory appropriately
…they mostly consult stuff that lives on the subtype
… just declaring them as a no-op
Hamms
force-pushed
the
subtype-results
branch
from
259cc2d
to
7e4c397
Compare
There was a problem hiding this comment.
The idea is that results is looking into a lot of maze state, because that's where we have the concept of success or failure as it relates to maze state.
That sounds reasonable. I do wonder whether those failure modes are useful in the standalone repo as well.
|
I based this split off of the assumption that they were not; if we discover that assumption was false, it shouldn't be too hard to do some rearranging afterwards |
Where Subtypes preserve all those bits of logic that care about the actual gameplay of the subtype (where you can move, what things you can do, how you manipulate the state of the game) and ResultsHandlers preserve all those bits of logic that care about success/failure handling and feedback (which game states count as passing, what feedback we show based on what game state, etc)