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

Suggestion for dynamic screen update #34

Merged
merged 2 commits into from
Oct 8, 2024
Merged

Conversation

CizBargah
Copy link
Contributor

  • Use os.system('cls') to update
  • Edit : Ignore __pycache/ in .gitignore

- Use os.system('cls') to update

Ignore __pycache/ in .gitignore
- On setup.py, optimize code, add pick_from_cards(cards, suggestion to rewrite set_up_decks), remove pick_from_cards(cards)

On simulate.py, fix bug from incorrect difficulty, minor changes, suggestion to consider scenario where players tie

On gameplay.py, minor changes
Copy link
Owner

@maryjess maryjess left a comment

Choose a reason for hiding this comment

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

  • Will continue the review for lines 40-47 on setup.py
  • Will continue review for simulate.py

Both will be done soon.
Thank you!

gameplay.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
@maryjess maryjess self-requested a review October 7, 2024 15:00
@maryjess maryjess assigned maryjess and unassigned maryjess Oct 7, 2024
Copy link
Owner

@maryjess maryjess left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @CizBargah !
Overall:

  • new bug detected -- if the user enter incorrect difficulty for the second time, the code will continue to set up card with incorrect difficulty
  • new issue -- end situation, what if tie condition?

setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
simulate.py Show resolved Hide resolved
simulate.py Show resolved Hide resolved
simulate.py Show resolved Hide resolved
simulate.py Show resolved Hide resolved
simulate.py Show resolved Hide resolved
@maryjess maryjess merged commit 8d85d80 into maryjess:master Oct 8, 2024
4 checks passed
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