Closed GoogleCodeExporter closed 8 years ago
Original comment by steven.h...@gmail.com
on 6 Jan 2010 at 6:40
I've committed the code for a first stab at this feature. It seems to be pretty
stable right now; I had one crash, but couldn't replicate. If anyone who tests
this
out runs into any problems, let me know!
Original comment by steven.h...@gmail.com
on 7 Jan 2010 at 4:08
Colored frames work brilliantly. It's fast and stable for the most part..I'm
pretty much in love with you :)
I am getting periodic crashes however. They appear to happen only when opening
up the HH window (not the
visualizer, the text one; though I haven't attempted to open the visualizer so
it may happen then as well)..but
again, it is pretty sporadic. I can't replicate it on command, but that's the
only time it happens the best I can
tell.
Also, when registering for tournaments, the colored frame is around both the
buy-in dialog and the
confirmation dialog. This should probably be eliminated, but I haven't had the
chance to look through the
code yet and take a crack at it.
Original comment by BuckNu...@gmail.com
on 7 Jan 2010 at 5:55
Thanks for the info, BuckNut! You're welcome to take a crack at the code, but
I'll
warn you: you're diving into the deep end. The window manager code is by far
the
screwiest section of the whole code base, and I've been meaning to clean it up
for a
while. I'm working on auto-PFR right now, but I'll take a look at these bugs as
soon
as I can.
Original comment by steven.h...@gmail.com
on 7 Jan 2010 at 6:07
I've committed a fix for the overlay on the tournament lobby and buy-in dialog.
I
didn't have time to buy-in to a tournament, but I think it should fix it for
that
too. I don't know if it'll fix the periodic crashes, but the function I'm using
instead is more robust, so perhaps that will do it. Give it a try!
Original comment by steven.h...@gmail.com
on 7 Jan 2010 at 7:06
I scanned through the code for the Window Manager - a code review I guess.
I have the following comments to make it more reliable. All comments to be
taken in
a friendly tone and only as suggestions :-)
* VERY IMPORTANT: The result of calling the AX methods such as
AXUIElementCopyAttributeValue are not being checked for error/success. It's
important to do so because the AX API is appallingly bad. The callbacks will
receive
notifications but when you query for the attribute values, the AXUIElementRef
value
your callback received might no longer be valid.
Adding paranoid level of error-checking to the AX method calls was important in
making Poker Copilot stable.
* The HKWindowManager.m is way too big. Almost 800 lines. If you can extract
self-
contained, highly testable methods to new classes, this file will be more
readable,
and easier to debug.
* windowIsTableAtOpening has a high cyclomatic complexity. (Nested
if-if-for-if-if
is hard to follow, hard to debug)
* windowDidOpen has three massive if-else blocks. Each block deserves a method.
* The heuristics for determining if a window represents a table window are
insufficient. It takes some trial and error to get this correct. Here's what I
do in Poker
Copilot to determine if a PokerStars window is a table window:
public boolean isTableWindow(String title) {
return StringUtils.countMatches(title, "-") > 1 && !title.startsWith("Tournament
Buy-in") && !title.startsWith("Chat:");
}
I hope you can read Java! A simple translation: There has to be more than one
hyphen
in the title, and the window title can't start with "Tournament Buy-in" or
"Chat:"
Original comment by steve.mc...@gmail.com
on 7 Jan 2010 at 9:09
Steven, are you running XCode 3.2? If so, I recommend doing static analysis
(from the
Build menu -> Build and Analyze).
It will help spot some bugs. For example, it showed me that getMainWindow can
return
an undefined value if the main window changes while it is looping.
Original comment by steve.mc...@gmail.com
on 7 Jan 2010 at 9:22
Steve, you read my mind
(http://code.google.com/p/blazingstars/issues/detail?id=13#c4)! :-) And thank
you
for taking the time to do a code review; that's something I really appreciate.
A major cleanup of HKWindowManager was definitely next on my to-do list, and I
agree
with a lot of what you said. I have some questions (and comments, for my own
future
reference), though, if you'll indulge me. Point by point:
* How are you dealing with failure when the AX API calls fail?
* I know I write code in a messier way than you're probably used to as a
professional software developer, so I hope you'll forgive the fact that BS will
probably never be a fully (or even partially, unless someone does it for me)
unit-tested program. I've written programs with unit-tests, and regression
tests,
and so on, but most of what I do is scientific coding, where if I could write a
test,
I wouldn't have to write the program. :-) I'm afraid I never really got into
the
habit, though it's a 2010 resolution to try harder...
* Yeah, I know that windowIsTableAtOpening is a mess. It, and windowDidOpen,
are
the first on my list.
* If I can read LISP and C++, I can probably handle any Java you'll throw at
me. :-)
But what window in PokerStars starts with "Chat:"? The only "chat" I know is the
chat box, but that doesn't show up in the AX hierarchy as far as I recall...
and I
think that the PS tournament tables have a buy-in screen that says "Tournament
Registration", not "Buy-in", don't they? I like the two-hyphen trick, though!
It
can only be a gatekeeper, since I still have to know if it's a tournament table
or a
cash table, but it's a nice little trick and I'll definitely use it.
Original comment by steven.h...@gmail.com
on 7 Jan 2010 at 9:51
Ooh, nice idea on the static analysis, I forgot that Xcode had that! I'll run
that
tomorrow...
Original comment by steven.h...@gmail.com
on 7 Jan 2010 at 9:52
* If an AX API call fails, I typically cease with the current window, and get
the next
one in the next list. In windowIsTable you could return HKNotTable if there is
an
error. Or use another constant...maybe HKErrorGettingAttributeValue.
* I wasn't thinking so much of unit tests - just looking into why WindowManager
might be crashing BS sometimes. Although I'm pretty rigorous with unit tests, I
know
that few developers are. Even those who call themselves professionals! :-)
What I was suggesting was that if you have smaller single-task methods or
functions, you can more easily locate the problems in this class. Unfortunately
XCode's refactoring is pretty lousy, so breaking up a large method into smaller
ones
is not so easy to do.
* Oops, looks like I copied Poker Copilot's code for checking Full Tilt windows
and
not PokerStars windows. Full Tilt can have detached chat, etc. The hyphen trick
works
though!
Original comment by steve.mc...@gmail.com
on 7 Jan 2010 at 11:40
Steven, activated this feature in the debug version and it works great. Another
good
job.... I ran 4 tables at once and it went to the active window every time in
the
course of 5 hours and 16 tournaments.........
Original comment by vyo...@gmail.com
on 12 Jan 2010 at 11:55
Closing this issue for feature freeze; please report bugs in a new issue.
Original comment by steven.h...@gmail.com
on 15 Jan 2010 at 12:43
Original issue reported on code.google.com by
steven.h...@gmail.com
on 6 Jan 2010 at 8:19