Missing Cards Used On PokerHandResult
hailwood opened this issue · 7 comments
I'm submitting a ...
- bug report
- feature request
- question about the decisions made in the repository
- question about how to use this project
Summary
Right now the cardsUsed
propery on PokerHandResult
is never populated.
This makes it difficult to work out more detail on the score. E.g. that player A scored Two Pair with Aces, whereas player B scored two pair with Threes.
Other information
I believe the fix for this is to update each new
statement here to pass through ranked[0]
as the third parameter https://github.com/mitch-b/typedeck/blob/master/src/services/pokerScore.service.ts#L160
And then updating the constructor here to pass use that parameter the same as cards
.
https://github.com/mitch-b/typedeck/blob/master/src/models/poker/pokerHandResult.model.ts#L27
Additional Request
While doing this, could we get an additional kickers
property added to output the additional information e.g. instead of just Two Pair
it would be great if we could go
`${score.toString()} ${score.kickers}`
To get Two Pair Ace Two
Those where the kicker is not relevant should just return an empty string..
Thanks for submitting this, @hailwood. Very high quality issue report.
I am a tad busy over the next few weeks, but I would gladly accept a pull request if you have time. Alternatively, I'd be more than happy to help tackle adding this in for you soon. 👍
Again, thanks for your thoughtful analysis.
What if we provided just an array of the cards which did not have an impact on the scoring (sorted in order of highest value), then you can print out as needed?
public get kickers (): PlayingCard[] {
const cardsNotUsedInResult =
this.cards.filter((c) => this.cardsUsed.indexOf(c) === -1)
.sort((a, b) => this.gameType.rankSet.getRankValue(b) - this.gameType.rankSet.getRankValue(a))
return [...cardsNotUsedInResult]
}
Which could then be consumed by:
`${score.toString()} ${score.kickers.map(card => CardName[card.cardName]).join(' ')}`
For such scenarios:
Cards in Play: [Seven of Spades , Seven of Diamonds , Nine of Spades , Ten of Diamonds , Nine of Hearts]
Cards Used in Hand: [Nine of Spades , Nine of Hearts , Seven of Spades , Seven of Diamonds]
Hand Type: Two Pair
=> Two Pair Ten
Cards in Play: [Seven of Spades , Eight of Diamonds , Nine of Spades , Ten of Diamonds , Jack of Hearts]
Cards Used in Hand: [Jack of Hearts , Ten of Diamonds , Nine of Spades , Eight of Diamonds , Seven of Spades]
Hand Type: Straight
=> Straight
Cards in Play: [Three of Spades , Two of Diamonds , Ten of Spades , Four of Diamonds , Eight of Clubs]
Cards Used in Hand: [Ten of Spades]
Hand Type: High Card
=> High Card Eight Four Three Two
Cards in Play: [Seven of Spades , Nine of Clubs , Nine of Spades , Nine of Diamonds , Nine of Hearts]
Cards Used in Hand: [Nine of Clubs , Nine of Spades , Nine of Diamonds , Nine of Hearts]
Hand Type: Four Of A Kind
=> Four Of A Kind Seven
That wouldn't actually print out the CardName
property that had a play in the scoring, though. However, that is now properly stored in cardsUsed
. We could improve surfacing the description of the winning hand though based off the PokerHandType
. If we did that though, what would be the expected output if winner is Straight/Flush? Would you like to see:
Straight Jack Ten Nine Eight Seven
Or, if Three of a Kind of 9s, would you want to see this (including kickers King/Jack):
Three Of A Kind Nine King Jack
It's possible to have some function like this:
public get scoringHandCardNames (): CardName[] {
const sortedCardsUsed =
this.cardsUsed.sort((a, b) => this.rankSet.getRankValue(b) - this.rankSet.getRankValue(a))
const uniqueCardNames = new Set<CardName>(sortedCardsUsed.map(c => c.cardName))
return [...uniqueCardNames]
}
Which you could then use like:
`${score.toString()} ${score.scoringHandCardNames.map(cn => CardName[cn]).join(' ')} ${score.kickers.map(card => CardName[card.cardName]).join(' ')}`
To produce (with same example above):
Cards in Play: [Seven of Spades , Seven of Diamonds , Nine of Spades , Ten of Diamonds , Nine of Hearts]
Cards Used in Hand: [Nine of Spades , Nine of Hearts , Seven of Spades , Seven of Diamonds]
Hand Type: Two Pair
=> Two Pair Nine Seven Ten
Cards in Play: [Seven of Spades , Eight of Diamonds , Nine of Spades , Ten of Diamonds , Jack of Hearts]
Cards Used in Hand: [Jack of Hearts , Ten of Diamonds , Nine of Spades , Eight of Diamonds , Seven of Spades]
Hand Type: Straight
=> Straight Jack Ten Nine Eight Seven
Cards in Play: [Three of Spades , Two of Diamonds , Ten of Spades , Four of Diamonds , Eight of Clubs]
Cards Used in Hand: [Ten of Spades]
Hand Type: High Card
=> High Card Ten Eight Four Three Two
Cards in Play: [Seven of Spades , Nine of Clubs , Nine of Spades , Nine of Diamonds , Nine of Hearts]
Cards Used in Hand: [Nine of Clubs , Nine of Spades , Nine of Diamonds , Nine of Hearts]
Hand Type: Four Of A Kind
=> Four Of A Kind Nine Seven
Sorry I know this is a lot - but it's kind of difficult to understand everyone's use cases. I am going back and forth on this scoringHandCardNames
because if needed, it's something that could easily be implemented on the consumer's application. Especially if you're extending this to some other game mode where the suit may come into play. Now the library is making more assumptions. What if we just stuck with surfacing cardsUsed
and kickers
? Open to your thoughts!
Hey @mitch-b,
Thanks for the detailed response. I've highlighted the main information of my reply below as bold.
Yeah I actually had a play myself and realized that it's more complicated than I had originally thought.
Where things get most difficult I think is around that output which I now believe agree it should be done on the consumer side.
Why?
Well because what we need to output is entirely dependent on the previous/next players hands, I.e. if both players have the same hand type, we need to output the cards used so we can see why one player beat the other. If it's a draw in primary, we need to output the next set of kickers so we can detail why the other user won again, and continuing on until the kickers are all gone.
But we only ever have access to that if the consumer calls scorePlayers
instead of individually scoring a player or a hand. So it makes sense to delegate this to the consumer.
I feel that passing through the entire ranked set PlayingCard[][]
is the way to go here since it gives us a lot of information, and the ranking/grouping is determined by the game mode. However there seems to be a bug in what cards from the hand are discarded if they're not a primary win.
E.g. this hand here https://github.com/mitch-b/typedeck/blob/master/src/services/pokerScore.spec.ts#L23 I can't remember the other card, but I know the Eight gets dropped, when it should be the Two getting dropped which ends up making the remainder of the ranked set useless. I attempted to resolve this, but I couldn't actually find where this was taking place.
--
Matt
I thought it might be helpful in someway if you had a fix for cardsUsed
at a minimum. I might need to take a bit and follow what you're saying above. Until then, feel free to check out feature/poker-hand-details
. Let me know if I was down a reasonable path there.
I think the bug you're seeing here (missing/dropped cards) is also set to be resolved with fix for #9.
Let me know if I'm off base with that assumption and I can take a closer look.
@mitch-b We're good to go here!
If you're curious, this is the project I'm using typedeck in https://github.com/hailwood/texas-holdem-scorekeeper
These updates that have surfaced the bugs/extra feature requirements mostly come from the scorePrinter here - https://github.com/hailwood/texas-holdem-scorekeeper/blob/master/src/lib/scorePrinter.ts
Great work on this package!
Happy to help. Glad you're finding some use from it! Feel free to submit any PRs as you see fit while using the library, or I can shed some light as to why things are the way they are if questions arise on how to use any objects. There's so much ...
Frankly, it existed because I wanted to learn TypeScript. If I could go back, I would've made sure I used semicolons! Oh, well, maybe a PR in the future ... 👍