Closed maosatgithub closed 2 years ago
OMG, this is amazing, and so very cool and very impressive. "Strategy + EV" now can have it's color encoding! great news. I will review it and test it before merging to gui branch
Thanks very much for your effort!
I will test it on mac,windows and linux so it might take some time before I merge.
Glad you like. Sure, take your time, and let me know in case you got any questions.
I also have some ideas for further improvement, e.g. the Rough Strategy should also show the negative EV colors as reference, and maybe it could also in addition split the combos numbers into +/- EV counts. But I wanted to share this first version first to get feedback.
Sorry, I noticed that with the 7th "Cleanup" commit I introduced a bug that caused display issues when a range got true zero. The added commit reverts that portion of the Cleanup.
Sorry, I noticed that with the 7th "Cleanup" commit I introduced a bug that caused display issues when a range got true zero. The added commit reverts that portion of the Cleanup.
Great. I will do some checks. I got to say that my UI code is truely ugly, the entire UI code is written hasted, I can imagine how hard it is to read and work on this pile of code.
I'm checking your changes locally. The code works prefectly generally. However I find the UI rather wired in some case, for example:
when the range is close to zero, the entire strategy somehow "collapse" to a line, it would be very hard for human eye to tell what is what.
I'm thinking how to solve this problem. Currently I came up with an possible idea and I would like to discuss it with you.
The idea is to add another checkbox to allow the user to select whether to show strategy with or without range information, with the box checked, the strategy would show as what you committed (with gray area indicating range), and if the box is unchecked, the UI would work as before (without gray area).
I'm not sure whether it's a good idea, or if there are other ways.
Yes, I noticed that downside myself. As an alternative (or addition) to such a switch I was thinking about a zoom feature, where instead of scaling to the absolute range, it could be done just relative, e.g. to the maximum. That probably would work very well for the detailed view, but on the left table, there might be a few hands that still have close to full range and that would limit the amount of zoom. To mitigate, they could be also be zoomed further, with some indication that that happened. Another possibility would be to just put a lower limit to the remaining area for the strategy drawing. E.g, leave like at minimum 5 pixel lines, or like 10% of the box, even if the actual range is still lower. So 90% of the box would be used to scale according to range, and the lower 10% would be also there. That probably needs some explanation, as it is not intuitive.
Yes, I noticed that downside myself. As an alternative (or addition) to such a switch I was thinking about a zoom feature, where instead of scaling to the absolute range, it could be done just relative, e.g. to the maximum. That probably would work very well for the detailed view, but on the left table, there might be a few hands that still have close to full range and that would limit the amount of zoom. To mitigate, they could be also be zoomed further, with some indication that that happened. Another possibility would be to just put a lower limit to the remaining area for the strategy drawing. E.g, leave like at minimum 5 pixel lines, or like 10% of the box, even if the actual range is still lower. So 90% of the box would be used to scale according to range, and the lower 10% would be also there. That probably needs some explanation, as it is not intuitive.
Yes, I agree, this would also cause confusion. I wonder what piosolver or other commercial solvers do in this situation.
Yes, I agree, this would also cause confusion. I wonder what piosolver or other commercial solvers do in this situation.
I don't have any experience myself with those, I would need to watch some YouTube videos to get an idea. From what I recall from seeing so far, they might use a lot of more text. So on in that sense, one means to mitigate the effect is to include more text information in the detailed box. E.g. add the range number to both strategy views, and add the action-percentages also to the "strategy+EV" view.
What should also be consider is that when the range or either opponent is approaching zero (and definitely when it is zero), the solver results for (that hand of) that branch of the tree are very likely not valid anyhow. That was one of the initial motivation for me, not wanting to see those likely invalid results. Because I was seeing e.g. a lot of raises for total junk on the river, but it turned out, that the range for that was zero already. So those "raises" were not part of the solver estimations anyhow. Also it is in indication that you really should not be in that branch of the game tree with those hands listed with a practical range of zero.
That said, I played a little bit with the ideas I mentioned earlier, and now find a combination of a very slight increase of the minimum display area, and an automatic zoom function for the current field of the left table quite compelling: For the single field in the left table, were currently the detailed view is shown for, it is zoomed in as if the range would be 100%.
I will add an commit for that next, and I am looking forward for your comments. One thing you want to look into, is whether to display the available actions in the summary according to distribution (that's what I implemented now), or whether a it should be ensured, that any kind of hidden fold/non-fold should be shown. Have a look at AJo, JTo, A3s and K3s in the example view you've show to see what I mean, but make sure that you look also at the ranges, to understand why it is shown this way.
That said, I played a little bit with the ideas I mentioned earlier, and now find a combination of a very slight increase of the minimum display area, and an automatic zoom function for the current field of the left table quite compelling: For the single field in the left table, were currently the detailed view is shown for, it is zoomed in as if the range would be 100%.
So those "raises" were not part of the solver estimations anyhow. Also it is in indication that you really should not be in that branch of the game tree with those hands listed with a practical range of zero.
About those invalid raises, I agree 100%, it also concerns me for quite a long time. and in the old version you will have to switch between range and strategy many times to find out which one is valid and which one is not, it's kind of annoying.
I checked out the zoom feature you mentioned, and I really like it a lot. I makes things a lot clearer.
About the available actions thing. I think the current status is fine. I think the "rough strategy" give the user pretty good idea what the available actions are, and I'm pretty sure this is how piosolver and other commercial solvers deal with this situation.
I think this PR is good to merge now, I will wait for your confirm, if you have nothing else to commit, I will merge this to gui branch, and in the next release, the changes will be push to master.
Thanks for all your efforts man!
I don't have any stale commits at the moment, so please go ahead with merging. I will set up new PRs for further stuff as I get to it, like the ideas for the rough strategy window we discussed. If it is ok, I will likely also try to refactor a little bit of the UI code, at least reduce the amount of copy/past by creating support functions that can be used in different places, and also by introducing more constant labels for parameters like colors used. But the thanks comes from my side as well, as said before for providing the solver at all, and now also for integrating my changes, so I don't have to maintain them separately!
I don't have any stale commits at the moment, so please go ahead with merging. I will set up new PRs for further stuff as I get to it, like the ideas for the rough strategy window we discussed. If it is ok, I will likely also try to refactor a little bit of the UI code, at least reduce the amount of copy/past by creating support functions that can be used in different places, and also by introducing more constant labels for parameters like colors used. But the thanks comes from my side as well, as said before for providing the solver at all, and now also for integrating my changes, so I don't have to maintain them separately!
Great, thanks for that, and looking forward for more contribution from you!
Hi, first, thanks for providing the code base! Very much appreciated!
This is to improve the visualization of the results, by scaling the "Strategy" and "Strategy + EV" views with the actual range data of the current node. The range is also considered proportionally when aggregating the summary in the left table. So unlike before, actions (like fold) for hands that have been 100% folded before, do not show up any more. And finally, this introduces an color coding of the EV in the "Strategy + EV" views for Check/Call and Bet/Raises actions: The more negative the EV is, the more blue is added, thus indicating if something is a call is for value of for defending, or if a Bet/Raise is for Value, or a (Semi)-Bluff.
Same as "strategy" view:
For cosmetics, I changed the color coding in the "EV" (only) view to transition of grey, instead of over brown. I also increased the window size of the StrategyExplorer slightly, so that on Windows the left table view is seen in total.