Skip to content

Add a few tutorial challenges #411

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

Merged
merged 6 commits into from
Jun 22, 2022
Merged

Add a few tutorial challenges #411

merged 6 commits into from
Jun 22, 2022

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Jun 16, 2022

Start designing the tutorial (via challenges) so that we can figure out which new features are necessary.

Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

I like it so far, but I think we should definitely address #387 before things get out of hand. 😄

I think #378 will be particularly helpful for later tutorial challenges, so we can sequence things like "go get at least 10 trees". "OK, good, now that you have some trees, craft them into ..." "Good job, now you need some water". etc.

@xsebek
Copy link
Member Author

xsebek commented Jun 17, 2022

Sure I think those two tickets would be nice, but they can be worked around and then fixed easily.

I am more interested in issues that will come up that I can not work around, like missing a know command for checking scan and update.

@xsebek xsebek mentioned this pull request Jun 19, 2022
// a <- flowerAt (2,-1);
// b <- flowerAt (1,-1);
// c <- flowerAt (1,-2);
// return (a || b || c);
Copy link
Member Author

@xsebek xsebek Jun 19, 2022

Choose a reason for hiding this comment

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

I have no idea why this does not work. When I manually tested it, it returned true just fine:
image
The second line is the entire win copy-pasted.

@xsebek
Copy link
Member Author

xsebek commented Jun 19, 2022

@byorgey @TristanCacqueray @lsmor When you have time, could you please playtest these few tutorial challenges?

They might be too hard or explain things too much or not explain the goal enough or all of that. 😅

The debugging challenge might be critical to help with later challenges - namely the next one - it does not work yet, but any suggestions are welcome. 🙂

@xsebek xsebek marked this pull request as ready for review June 19, 2022 20:49
@xsebek xsebek changed the title WIP: start tutorial Add a few tutorial challenges Jun 19, 2022
@byorgey
Copy link
Member

byorgey commented Jun 20, 2022

I note that tutorial 5 (build) should probably use known entities, since scan hasn't been introduced yet.

@byorgey
Copy link
Member

byorgey commented Jun 20, 2022

Things in TUTORIAL.md that aren't yet in any of the tutorial challenges here, that probably should make their way into this version of the tutorial as well:

  • Types (type signatures, type errors, etc)
  • Explaining the relationship between devices + commands
  • def
  • Bind notation (e.g. r <- build {...}; ...)
  • view
  • give

@byorgey
Copy link
Member

byorgey commented Jun 20, 2022

When I start the build challenge after completing the previous one, compass is highlighted instead of Goal. I think there is some state relating to the inventory list that is being persisted across games that really shouldn't be.

Copy link
Collaborator

@TristanCacqueray TristanCacqueray left a comment

Choose a reason for hiding this comment

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

It's looking really good, thanks!

@xsebek
Copy link
Member Author

xsebek commented Jun 20, 2022

@byorgey regarding the things that should be in the tutorial:

  • types - Yeah, that should be explained, but how do you direct players to try it in challenges? There is no way to test that.
    I guess the goal would have to say something like "Win by place "win" when you have tried it."
  • devices and commands - Good point, it is heavily hinted at, but I will make it explicit somewhere.
  • def - I do not think that is necessary for current challenges, we are at worst exploring a 3x5 area.
    What would it be useful for before you set out on longer journeys or do more complicated tasks?
  • bind notation - I could explain that using the grab command before the debugging challenge (which will need view r).
  • view - will be the core of debugging challenge: build a robot with bind, send it to crash, then view it and you will see how to win in its log.
  • give - good point, but I will leave a challenge that uses give base x for later - maybe that could be tied together with bind notation.

Sorry for the long wall of text, I will probably get back to this tomorrow, so it's also for me to not forget. 😅


Note to self:

  • Try to split grab&place into grab (TTTTTT.<) and place (.....T.<) challenges

@byorgey
Copy link
Member

byorgey commented Jun 20, 2022

  • Re: types: not everything has to be tested, presumably players want to learn about the game. 😄 But yes, I think something like "Win by place "win" when you have tried it" can work great.
  • Re: def, I agree, I was not suggesting we should incorporate def into the existing challenges so far. Just making a list of things that should eventually be included somewhere.

@xsebek
Copy link
Member Author

xsebek commented Jun 22, 2022

With a total of 10 tutorial challenges, I have run out of energy to write more.

I am leaving a proper tutorial of grab and def for later.

Once #464 is merged, I will rebase this PR and move goal descriptions to use the the newer goal from #450.

After that, I would like to merge this PR. Hopefully, only the wording will now require changes.

@xsebek xsebek requested a review from TristanCacqueray June 22, 2022 15:04
@xsebek xsebek requested a review from byorgey June 22, 2022 15:04
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

This is looking fantastic! Just a few small comments/suggestions.

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Jun 22, 2022
@xsebek
Copy link
Member Author

xsebek commented Jun 22, 2022

Btw. the new goal view looks great with my long tutorial descriptions, thanks @byorgey!

@xsebek
Copy link
Member Author

xsebek commented Jun 22, 2022

Weird, all tests failed on the crasher test, but I can not reproduce that locally:

$ cabal run swarm-integration -- -p crasher
Up to date
Tests
  Test scenario solutions
    Tutorial
      crasher: OK (0.21s)

All 1 tests passed (0.21s)

@byorgey
Copy link
Member

byorgey commented Jun 22, 2022

Strange, I can't reproduce it locally either...

@xsebek
Copy link
Member Author

xsebek commented Jun 22, 2022

I spent half an hour copypasting the commands from CI, to get a docker container as close as possible to our tests...

...can not reproduce there. 😠


I have no idea what is going on, so I will try this:

  1. push an almost empty commit to restart the CI and if it does not pass continue to 2.
  2. push another branch that will be a copy of this one and if it does not pass continue to 3.
  3. mark the test as expected to fail in CI

} { return false }
solution: |
crasher <- build {move; move; move; log "bye"; move};
wait 1000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could a lower or higher wait value help here?

Copy link
Member Author

@xsebek xsebek Jun 22, 2022

Choose a reason for hiding this comment

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

I doubt that - the secret robot has only 15 numbers to iterate over and will get to the crashed robot shortly.
A lower number would not change anything, I think.

FWIW: wait 5 is the necessary value currently, so 1000 should be enough 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant, because the test timeout, perhaps a lower value would make the win condition works better? In case the test instance is overloaded or something... Also it seems like the test timeout after 1 second by default, so maybe pushing that to 2 second could help too?

Copy link
Member Author

Choose a reason for hiding this comment

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

You know it does actually take 0.44s to run the test, unlike the others which take ~0.10s.

Copy link
Member Author

@xsebek xsebek Jun 22, 2022

Choose a reason for hiding this comment

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

I pushed to and restarted #465, let us see... 👀
EDIT: alternative test to turn off check in CI in #466.

@xsebek xsebek removed the merge me Trigger the merge process of the Pull request. label Jun 22, 2022
@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Jun 22, 2022
@xsebek
Copy link
Member Author

xsebek commented Jun 22, 2022

I got to step 3:

  1. mark the test as expected to fail in CI

And do not intend to spend any more time on this absurd CI mystery until I can reproduce that locally.
The test will still run in CI, but failure will be ignored with a nice message for the future.

@mergify mergify bot merged commit 12344cd into main Jun 22, 2022
@mergify mergify bot deleted the start-tutorial branch June 22, 2022 20:58
@byorgey
Copy link
Member

byorgey commented Jun 25, 2022

@xsebek I have a new theory why that test wasn't working in CI: the 09-secret.sw file was not listed in the data-files field in the .cabal file. When running the test locally, the file existed. But when running the test in the CI environment, it unpacked the source from the Cabal package in order to have a clean test environment, so that file did not exist. The run failed, and it then timed out. I will try pushing a fix along with a related PR in just a minute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants