charlestudor / PokerNowLogConverter

A simple CLI tool for converting logs from Poker Now games to the PokerStars format.
MIT License
14 stars 4 forks source link

Possible issue with names with parentheses. #7

Closed elireznekervitz closed 1 year ago

elireznekervitz commented 2 years ago

Hi thank you for creating this. I don't much coding experience at all.

I am trying to run this.

python3 -m poker_now_log_converter -H (eli)

zsh: missing end of string

Is there a way to get it to recognize names with ()?

charlestudor commented 2 years ago

Hi! I'm sure that will be possible, thanks for raising the issue.

I'll take a look at this when I next get a chance.

charlestudor commented 2 years ago

Hey again!

I just had a quick try with a few names with parentheses, but the examples I used didn't seem to produce the error.

Please could you let me know an example name which results in the error, or upload an example log which is broken to this thread - that will help me fix it in the fastest time possible.

If it's urgent, you might be able to workaround the issue by using a Find All -> Replace process in a text editor, and change the name of the player with brackets.

Many Thanks!

elireznekervitz commented 2 years ago

Thanks for getting back to me! I am very new to running scripts and python so it could very well be an issue with something I am doing.

On Sat, Oct 15, 2022 at 10:15 Charles Tudor @.***> wrote:

Hey again!

I just had a quick try with a few names with parentheses, but the examples I used didn't seem to produce the error.

Please could you let me know an example name which results in the error, or upload an example log which is broken to this thread - that will help me fix it in the fastest time possible.

If it's urgent, you might be able to workaround the issue by using a Find All -> Replace process in a text editor, and change the name of the player with brackets.

Many Thanks!

— Reply to this email directly, view it on GitHub https://github.com/charlestudor/PokerNowLogConverter/issues/7#issuecomment-1279754425, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEKWR6I4CKYSSBLTJJYLOHTWDK36TANCNFSM6AAAAAARFYCQYY . You are receiving this because you authored the thread.Message ID: @.***>

-- Sent from Gmail Mobile

elireznekervitz commented 2 years ago

I will certainly try again and provide you with the steps I’m taking and messages I’m getting. Thanks for your help.

On Mon, Oct 17, 2022 at 11:33 Eli @.***> wrote:

Thanks for getting back to me! I am very new to running scripts and python so it could very well be an issue with something I am doing.

On Sat, Oct 15, 2022 at 10:15 Charles Tudor @.***> wrote:

Hey again!

I just had a quick try with a few names with parentheses, but the examples I used didn't seem to produce the error.

Please could you let me know an example name which results in the error, or upload an example log which is broken to this thread - that will help me fix it in the fastest time possible.

If it's urgent, you might be able to workaround the issue by using a Find All -> Replace process in a text editor, and change the name of the player with brackets.

Many Thanks!

— Reply to this email directly, view it on GitHub https://github.com/charlestudor/PokerNowLogConverter/issues/7#issuecomment-1279754425, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEKWR6I4CKYSSBLTJJYLOHTWDK36TANCNFSM6AAAAAARFYCQYY . You are receiving this because you authored the thread.Message ID: @.***>

-- Sent from Gmail Mobile

-- Sent from Gmail Mobile

charlestudor commented 2 years ago

Hi Eli, no problem at all! This tool isn't quite battle-hardened yet, so there's a good chance the problem is with the tool and not with what you're doing.

If you get stuck just reply to this thread with the commands you're using and the error message. If you attach the logs it will certainly help, but keep in mind we'll be able to see your cards, you could cut it down to a single hand in a text editor if you're afraid of giving away your strategies!

elireznekervitz commented 2 years ago

I'm not worried about my strategies lol, i'm not that good any way.

I will attach the logs i'm trying to use. I keep getting zsh: missing end of string when trying to set player name. Player name is (eli). Also it could be an issue with how python is set up on my computer.

I would definitely find this tool useful if i can get it to work. I just want to save my hands so I can review them later.

poker_now_log_pglV3eELNmK29yjk-K5NoMwT4.csv

Also if it works for you, if you might have time to meet through google meet or zoom to help me get set up, it would be much appreciated. I could even give you a tip for your time.

charlestudor commented 2 years ago

Hey Eli, haha neither am I don't worry!

I've been able to replicate, thanks very much for the log.

It looks like it is a problem with parenthesis when using bash or zsh (which is default on macOS now I believe). Your options are to either escape the parentheses manually with backslashes like -H \(eli\) , or you should be able to wrap the hero name in double quotes as well: -H "(eli)".

Let me know if this fixes the issue, thanks!

The log you provided actually highlighted another issue though, lack of straddle support! I'll get this sorted soon, I just need to work out what straddles are actually supposed to look like according to Pokerstars / HoldEmManager.

Cheers, -C

DrkYoda commented 2 years ago

You might also want to remove all Unicode characters from player names because Holdem Manager 3 does not like non ASCII characters to be in player name, I'm not sure if Poker Tracker plays nice with UTF.

charlestudor commented 2 years ago

@elireznekervitz Just added a new version v0.0.8 which supports straddle bets, so give it a go with the double quotes and let me know if it works, thanks!

@DrkYoda Good point! I'll do some testing with some non-ASCII / UTF variants and pre-process them out where necessary, thanks for the tip.

DrkYoda commented 2 years ago

@charlestudor actually now that I've thought about it more, I could be completely mistaken. The unicode character problem that I had was for a hand history converter that I made, but my converter translates the log into a JSON format called Open Hand History OHH. When the ohh file is created it puts all non ascii characters in a \u escape sequence, so "She’s cake" gets saved as "She\u2019s cake". So why the hell did HM3 have a temper tantrum about that? There are no non ascii characters in the name "She\u2019s cake". Maybe because of the forward slash? OK now I'm curious, I'm going to go back to an older version of my hand converter and set json.dumps(insure_ascii = false) so the Unicode character will be saved in its original form and then try to import the hand into HM3...I'll report back on what happens but I'll bet it will import the hand without errors.

DrkYoda commented 2 years ago

BTW, I really like your hand converter, the code is clean and easy to understand...very classy. Unlike the code for my hand converter which doesn't define a single method, much less a class. It's twisted knot of for loops and if/else statements in a linear progression of steps from top to bottom...its ugly but it works. I really need to refactor the code before it becomes unmanageable.

charlestudor commented 2 years ago

Thanks! To be honest when I opened the code earlier to look at this issue my first thought was "man this is a mess!". I think it's pretty hard to keep code for parsers like this organised with so many little edge cases and variations to deal with.

I've heard about the OHH format, I actually put it in the readme as an "on the way" feature, but I never got around to it since nobody got in touch to say they need it, and I just import straight into PT4. Nice to have a real spec though rather than having to guess how PokerStars outputs things.

Yes, please let me know what you find out with your Unicode tests, the more info we have about these undocumented things the better!

DrkYoda commented 1 year ago

Your code is very clean and well documented by my standards. For purposed of comparison you can take a look at the code in my hand converter here. Its current state is the stuff that nightmares are made of.

I agree, its nice to finally have a general specification for hand histories, and json is the natural format for the specification, but its a shame that so few people know about it much less, use it. That being said, there are some problems with the ohh standard.

  1. It produced files that are bloated with information that is common to all of the hands because the information gets repeated in every ohh object.
  2. Its an invalid json format because each hand history is a self-contained JSON object seperated by a blank line
    
    {"ohh": <standardized_hand_history_object_1>}

{"ohh": }

{"ohh": }

In order to make it valid json you just need to put the ohh objects in an array and seperate them with commas.

[ {"ohh": }, {"ohh": }, {"ohh": } ]

but this is not valid ohh.  So why have they done this?  Well, I'm not sure but I am sure that the justification given in the documentation is utter nonsense.
3. HM3 is also not entirely compliant with the ohh standard, there are optional name/value pairs that HM3 thinks are mandatory.  Most notibly, the `is_allin boolean` which the standard explicitly says is optional except for actions that result in a player actually going all in.  The standard makes sense, because most actions don't result in a player going all in, and some actions like checking, folding, and showing cards it is impossible to go all in, yet HM3 requires is_allin to be in every single action object so you end up with objects like

{ "action_number": 0, "player_id": 1, "action": "Check", "is_allin": false }


Really?  Why is `"is_allin": false` necessary to include? When you play at a live game, has anybody ever said "I check, and I'm not all in"?

Okay, I'll stop ranting about things that annoy me to strangers.  :p
charlestudor commented 1 year ago

Haha, I think it's funny how in software some perhaps throwaway decision made decades ago like json line spacing can cause developers no end of grief forever and ever onwards., despite being in such a niche area like poker hand representations!

Your code isn't as bad as you were saying - there's something to be said for simplicity and having a main.py file that just does the job. I actually wrote a first version of this tool which used regular expressions as well, but I abandoned it as the PN log format was changing every few weeks back then, and trying to get my brain to remember how the regexs worked and which needed changing / multiple variations was just impossible. My much slower solution of using the builtin 'in' operator does the job, eventually! (I also gave up trying to support the earliest versions I have logs for, as there are just too many tiny differences to account for!)

charlestudor commented 1 year ago

Closing issue. If this wasn't resolved Eli please don't hesistate to reopen or make a new ticket.

-C