santiweight / poker-base

BSD 3-Clause "New" or "Revised" License
4 stars 0 forks source link

Hand and ShapedHand #12

Closed tonyday567 closed 3 years ago

tonyday567 commented 3 years ago

Just saw a comment from you on these names.

I like Hole as the name for the two cards in a players hand, and Hand for what you are currently calling ShapedHand. I think that's closest to poker jargon.

santiweight commented 3 years ago

I agree with Hand -> Hole. However, I'm not so sure aboutShapedHand -> Hand. In particular, Hand is quite an overloaded term in poker I think. It can refer to:

I personally have come to accept that Hand is just too vague to use...

tonyday567 commented 3 years ago

Ok, fair enough. Their called starting hands also, but that sucks as a name, so ShapedHand is it.

My Enum instance for a ShapedHand:

https://github.com/tonyday567/poker-fold/blob/main/src/Poker/Types.hs#L443

It's convenient, because it slams it into a 13x13 matrix that I am going to use a lot. What do you think?

santiweight commented 3 years ago

Hmmm

I’m not sure. It’s definitely not “bad” but it certainly doesn’t make for the cleanest api. I am thinking of tearing out my custom enum instances.

How about toGridCoordinate :: ShapedHand -> (Int, Int) (13x13 coordinates) and the equivalent from. I think it would make this functionality more discoverable, and I can’t immediately think reason for using an Enum polymorphic function on a ShapedHand.

we could also just have both anyway, but this API is better IMO.

santiweight commented 3 years ago

Btw what do you think about ShapedHole? It strikes me as weird that there is no Hand, but there is a ShapedHand.

tonyday567 commented 3 years ago

Yeah, was just talking about the enum instance, not the API. You have a vision for the API you want and am happy to follow your lead. If enum instances are there at all, putting guards in them gets in the way of my use cases for them.

For the Range stuff you have, I use what might be called a ShapedRange, which is a 169-sized vector of Bools. ShapedHands for me are less about being a grid shape and more that the range of ShapedHands can be used to represent all sorts of things, such as betting strategies or range guesses.

Yes, it is a bit weird. Stick with the status quo.

santiweight commented 3 years ago

No problem at all - in case I didn't make it clear, I see no issue with including your Enum instances - let's definitely include them and I can always bring this up some other time!

santiweight commented 3 years ago

So your ShapedRange is, IIUC, a specialisation of Range. Is there an issue with Range that would make you apprehensive about using it? Perhaps we can switch to a more efficient version of Range and then unify those two tidbits?

santiweight commented 3 years ago

Oh - I think I understand now. Your ShapedHand is specialised to a Vector (I don't know which package is right for this use-case).

I think I want to have that too!

Could we incorporate your version of Range, since it is probably way faster and way better memory usage? As long as we have some way to map between the various Ranges, I would be way happier than what we have in the package right now!

newtype ShapedHandRange a = ...
newtype HoleRange a = ...
newtype HandRankRange a = ...

holeToShapedHandRange :: (a -> a -> a) -> HoleRange a -> ShapedHandRange a
-- you get the idea...

We could probably get something pretty working with data family, but the above addition would be a massive improvement

tonyday567 commented 3 years ago

Yep, instead of the Map you use. And sure can! HandRankRange might be too big for what you want (5k or so), but maybe not. It may not be a Vec Bool though - you might be looking for a probability.

santiweight commented 3 years ago

Yeah exactly - would you be okay with the polymorphic item type, or do you need that to be specialised also?

santiweight commented 3 years ago

I would just do ShapedHandRange Freq and you would do ShapedHandRange Bool I think.

tonyday567 commented 3 years ago

Yep, same thing. Just having the concept in base is enough for me.

tonyday567 commented 3 years ago

Intuitively, I am guessing that a Data.Vector.Storable with a size of 169 is what a RangedHole should be.

See https://github.com/tonyday567/poker-fold/blob/main/src/Poker/RangedHole.hs#L289

I think we just use newtype a storable vector rather than a numhask array.

santiweight commented 3 years ago

What do you think about naming - I prefer HoleRange? To me RangedHole sounds like you are constraining the type Hole.

tonyday567 commented 3 years ago

holerange sounds better, yeah.

On Fri, 22 Oct 2021 at 6:18 pm, santiweight @.***> wrote:

What do you think about naming - I prefer HoleRange? To me RangedHole sounds like you are constraining the type Hole.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/santiweight/poker-base/issues/12#issuecomment-949396211, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMMATLGGFXDYXCVH2HOSU3UIEM5TANCNFSM5FNJ6NXA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

santiweight commented 3 years ago

Closing in favour of #55