Avoiding drawing a new card from an era that will fail the tooClose check #121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The tooClose function will always hit if you draw too many early cards in the 1800-2020 range When the tooClose function rules out all cards, the game defaults to drawing a random card from the deck without any other failsafes. One of the failsafes in tooClose is to ensure cards are at least 1 year away from any existing card. Without this failsafe, we can have ties (which fail on a coinflip) and in some cases even a duplicate card being drawn. We also lose all the other balancing effects of avoidPeople and splitting cards into three different periods.
rowboat1
changed the title
tom-james-watson
added a commit
that referenced
this pull request
Jul 1, 2024
As pointed out in #121, it was previously possible for duplicate years or even duplicate cards to be drawn because we'd fall back to the random card choice too often. This was because the tooClose function could return true too easily for certain circumstances. This makes it so tooClose is slightly less strict, but also only applies that function as a second pass, meaning we should fall back to a set of relevant cards instead of the whole deck.
|
Hey Thanks for this PR - it's definitely a good attempt at fixing a bad flaw you point out. I do find the logic a bit hard to follow though, and think it's a little unnecessarily complex. It spurred me to make a similar, but simpler, PR that should also solve this issue: #122. Let me know what you think of that. |
|
I also found my own logic confusing, and was thinking up my own improved solution, haha! Let me know what you think of #123 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
tooClosefunction will always hit if you draw too many early cards in the 1800-2020 range.For instance, if you draw 1880, and then 2000, you will now cause any card in the 1800-2020 range to fail on the
tooClosecheck on your third draw, and force a random card to be drawn.When the tooClose function rules out all cards in a period, the game defaults to drawing a random card from the deck without any other failsafes.
One of the failsafes in
tooCloseis to ensure cards are at least 1 year away from any existing card. But this gets bypassed if the original check failed.Without this failsafe, we can have ties (which fail on a coinflip, and are annoying) and in some cases even a duplicate card being drawn.
This also creates many more happy cases that preserve the intended balancing effects of
avoidPeopleand splitting cards into three different periods.Should close #81 and #103 (in fact, if you look at the screenshot in #103 , you can see that they've drawn 3 cards in the 1800-2020 range during their game, likely causing this exact bug)