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

📜 New blackjack adventure #5331

Merged
merged 12 commits into from
Apr 16, 2024
Merged

📜 New blackjack adventure #5331

merged 12 commits into from
Apr 16, 2024

Conversation

MarleenGilsing
Copy link
Collaborator

@MarleenGilsing MarleenGilsing commented Mar 28, 2024

This PR should make the blackjack adventure actually doable for the students. :)

@Felienne (or other testers), would you mind playing the adventure yourself? I would love to know if my instructions are clear enough to make the adventure doable. It is level 17, so it's okay if it's a little hard.

I'd also like you to give your opinion on the first adventure. It is still quite long (and therefore a little ugly). The alternative is that I could split part 1 into 2 parts, with part 1 only being drawing a card maybe doing the calculating function and part 2 adding the rest. Pro: It does make the adventure smaller and more 'behapbaar', Con: It's less linear (now we only add parts underneath, if we choose the first option we'll have to add parts in the middle etc'. I liked the fact that we just add more code, in each adventure, so I chose for this format. But let me know if you prefer the alternative.

And as I told you on discord, the numbers for the cards are seen as string text, which isn;t weird, because they come from a list with other words like 'king'. This does cause the parser (if that is the right word) to add the numbers up asif they are words so 8+5 = 85 instead of 13. In the long and ugly code we have on the website currently, where I haven't used functions, this doesn't happen somehow. So this needs to be fixed before this PR can be merged.

Also, I haven't copied it to other languages yet, nor translated it to Ducth yet, because I want it to be right in ENG first.

@Felienne
Copy link
Member

Felienne commented Apr 2, 2024

Some notes here too:

  • fucntion -> function

I agree step 1 is very long, could we split it in two, where we first only program how to calculate cards? That can even be tested in isolation (I think in general trying to explain to kids how to test larger pieces of code is good!) If this remark makes no sense, leave it for now until we can have a meeting about it!

@MarleenGilsing MarleenGilsing marked this pull request as ready for review April 4, 2024 12:31
Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

Let's go!

Copy link
Contributor

mergify bot commented Apr 16, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 058df07 into main Apr 16, 2024
12 checks passed
@mergify mergify bot deleted the new_blackjack branch April 16, 2024 20:12
Copy link
Contributor

mergify bot commented Apr 16, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants