Skip to content
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

Update scores and bonus points at turn end #188

Merged
merged 4 commits into from
Aug 21, 2024

Conversation

yndajas
Copy link
Member

@yndajas yndajas commented Aug 15, 2024

This covers some more of #180

@yndajas yndajas changed the title Update answers and bonus points at turn end Update scores and bonus points at turn end Aug 15, 2024
@richpjames richpjames force-pushed the update-answers-and-bonus-points-at-turn-end branch 2 times, most recently from 2f52410 to 80c4a2b Compare August 19, 2024 14:50
Copy link
Member Author

@yndajas yndajas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! We don't seem to be using the output yet, as far as I can tell, and we're still manually getting stuff on turnEnd - can we start using the output instead?

Also, the WIPs need removing from the commit messages

server/models/round.ts Outdated Show resolved Hide resolved
server/models/round.ts Show resolved Hide resolved
server/models/round.ts Show resolved Hide resolved
server/utils/scoringUtils.ts Show resolved Hide resolved
server/models/round.ts Show resolved Hide resolved
server/machines/round.ts Show resolved Hide resolved
@richpjames richpjames force-pushed the update-answers-and-bonus-points-at-turn-end branch 4 times, most recently from d3fddfe to cc86dda Compare August 20, 2024 09:09
@richpjames richpjames marked this pull request as ready for review August 20, 2024 09:09
Copy link
Member Author

@yndajas yndajas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Mostly just a few suggestions, though there's one test assertion that needs fixing

server/utils/scoringUtils.test.ts Show resolved Hide resolved
server/utils/scoringUtils.ts Show resolved Hide resolved
server/models/round.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.test.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.test.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.test.ts Outdated Show resolved Hide resolved
server/utils/scoringUtils.test.ts Show resolved Hide resolved
server/utils/scoringUtils.test.ts Outdated Show resolved Hide resolved
@richpjames richpjames force-pushed the update-answers-and-bonus-points-at-turn-end branch from cc86dda to aeb9730 Compare August 20, 2024 14:45
Copy link
Member Author

@yndajas yndajas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Just one thing about the git history, but otherwise I think it's good to go. Nice work!

(I can't approve it because I opened the PR)

server/utils/scoringUtils.test.ts Show resolved Hide resolved
server/utils/scoringUtils.test.ts Show resolved Hide resolved
yndajas and others added 2 commits August 21, 2024 16:00
This is the correct way to output data from a machine when it reaches
its final state.
See docs: https://stately.ai/docs/final-states#output
@richpjames richpjames force-pushed the update-answers-and-bonus-points-at-turn-end branch from aeb9730 to be338ad Compare August 21, 2024 16:17
yndajas and others added 2 commits August 21, 2024 17:26
The logic here is relatively complex so worthwhile testing.
I have opted to only test the exported functions as the smaller, local
ones are very simple.

Co-authored-by: Rich James <[email protected]>
This is just useful for logging to see if the interactions in the model
are working as expected. Depending on the guard we will either play
another turn or end the round.
@richpjames richpjames force-pushed the update-answers-and-bonus-points-at-turn-end branch from be338ad to 1f086f6 Compare August 21, 2024 16:26
@richpjames richpjames merged commit b3172c4 into main Aug 21, 2024
3 checks passed
@richpjames richpjames deleted the update-answers-and-bonus-points-at-turn-end branch August 21, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants