Missing Cards Used On PokerHandResult

hailwood commented 6 years ago

I'm submitting a ...

[x] bug report
[x] 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..

mitch-b commented 6 years ago

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. 👍

mitch-b commented 6 years ago

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!

hailwood commented 6 years ago

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

mitch-b commented 6 years ago

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.

mitch-b commented 6 years ago

I think the bug you're seeing here (missing/dropped cards) is also set to be resolved with fix for mitch-b/typedeck#9.

Let me know if I'm off base with that assumption and I can take a closer look.

hailwood commented 6 years ago

@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!

mitch-b commented 6 years ago

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 ... 👍

mitch-b / typedeck