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.

Sign up for GitHub

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

Jump to bottom

Grant RobotIntoWater achievement #1504

Merged
merged 7 commits into from Sep 11, 2023
Merged

Grant RobotIntoWater achievement #1504

merged 7 commits into from Sep 11, 2023

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Sep 10, 2023 •
edited

Towards #1435. Some refactoring, + grant RobotIntoWater achievement when a robot dies in the water.

@byorgey byorgey added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Sep 10, 2023
@byorgey byorgey requested a review from kostmo September 10, 2023 00:35
@kostmo
Copy link
Member

kostmo commented Sep 10, 2023

Heads up on the build error: #1505

kostmo
kostmo reviewed Sep 10, 2023
View reviewed changes
src/Swarm/Game/Step.hs
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to have an integration test for this (see #1507), but in lieu of such a framework, can you add a "test plan" to the toplevel PR description?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kostmo good idea. How's this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Following a suggestion from @xsebek I added an actual integration test.

@kostmo kostmo mentioned this pull request Sep 10, 2023
Open
4 tasks
@xsebek
Copy link
Member

xsebek commented Sep 10, 2023

If the achievements are part of GameState, then you can test it in testSolution' - the winning conditions can be something like:

  1. build a robot (robotNumbered 1 exists)
  2. drown it (robotNumbered 1 does not exist)

@byorgey
Copy link
Member Author

byorgey commented Sep 10, 2023

Good idea, done!

@restyled-io restyled-io bot mentioned this pull request Sep 10, 2023
Merged
@restyled-io @restyled-commits
Restyled by fourmolu (#1511)
010ee74 
Co-authored-by: Restyled.io <commits@restyled.io>
xsebek
xsebek approved these changes Sep 10, 2023
View reviewed changes
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

🏊‍♂️

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Sep 11, 2023
@mergify mergify bot merged commit 41c94f1 into main Sep 11, 2023
11 checks passed
@mergify mergify bot deleted the robot-into-water branch September 11, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostmo kostmo kostmo left review comments

@xsebek xsebek xsebek approved these changes

Assignees
No one assigned
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. merge me Trigger the merge process of the Pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@byorgey @kostmo @xsebek