adds new board to game over screen#870
Conversation
LandonPatmore
requested review from
a team and
zach-klippenstein
as code owners
|
|
LandonPatmore
force-pushed
the
lpatmore/samples/dungeon-update
branch
from
5988e9d to
04a7460
Compare
| } | ||
|
|
||
| sealed class Output { | ||
| object NewBoard : Output() |
There was a problem hiding this comment.
Seems like this doesn't need to be a sealed class if we do the same thing any time any type of Output is emitted!
There was a problem hiding this comment.
I went with this instead of just an object Output in the spirit of it being a sample that people can add to. I will foundations make the call here.
There was a problem hiding this comment.
We actively discourage this kind of things as a speculative YAGNI (you ain't gonna need it) violation. Definitely wouldn't want to see it in a sample.
I won't be able to give the PR a proper review until next week. In the meantime, thanks for the contribution!
| cancelable = false, | ||
| onEvent = { context.actionSink.send(restartGame()) } | ||
| onEvent = { | ||
| if (it is ButtonClicked) { |
There was a problem hiding this comment.
nit: I feel like if (it is ButtonClicked && it.button == NEUTRAL) { ... } else {...} would more closely replicate the original behavior
steve-the-edwards
left a comment
steve-the-edwards
left a comment
There was a problem hiding this comment.
LGTM with the change of Output to an object NewBoard
5 participants
closes #205