-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(ui): highlight shapes that can currently be placed #55
Conversation
@@ -198,6 +198,10 @@ class GameController : Controller() { | |||
} | |||
subscribe<HumanMoveRequest> { event -> | |||
logger.debug("Human move request") | |||
|
|||
val moves = GameRuleLogic.getPossibleMoves(event.gameState) | |||
logger.debug("total: ${moves.size}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the debug output or make the message more verbose so that it will be useful in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yeah, my bad.
I used to print the whole list before, but removed that because it was too noisy.
I'll update it; it's useful for debugging
@@ -198,6 +198,10 @@ class GameController : Controller() { | |||
} | |||
subscribe<HumanMoveRequest> { event -> | |||
logger.debug("Human move request") | |||
|
|||
val moves = GameRuleLogic.getPossibleMoves(event.gameState) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reduce that to a set of possible shapes here, so that the check below doesn't have to go through all moves for every shape?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I did that meanwhile.
Still, had to wait for the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the backend submodule should point to a commit where the new algorithm is used already, right?
yes, exactly, after the other one is merged. |
71e6647
to
99eb293
Compare
This doesn't work properly yet due to a bug. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
99eb293
to
3fcbf56
Compare
No description provided.