mrmin123
commented
7 years ago I feel like this might be an issue that might have to do with the Sikuli 1.1.1 upgrade. I'm pretty sure previously elif isinstance(text_ref, Match): worked properly, but now it's not triggering a match even though I've confirmed that the instance type of the text_ref object is of the Match class. I can't even get it to match the superclass Region. I could get it to work by using elif text_ref.__class__.__name__ == 'Match': but I try to avoid using two chained __ references.
I'm holding off on merging your code in for now since it's a workaround to get around the explicit handling that I prefer.
Also, throwing away the exception is by design, otherwise you'd have a crash and stacktrace every time your repair ports are empty (check the code), which would be silly. This is due to the nature of the for i in findAll() loop behavior where it crashes if the findAll() does not find anything. There's no danger of throwing away the exception to this if the downstream code works as intended, and if the downstream code is failing, it's easy enough to change the catch block for debugging purposes.
Defaulting to Match does not work because of the above issue where it actually does not match to Match even when it is Match. Adding a dummy value also just either delays the inevitable crash or makes the crash silent, which is not desired in that particular scenario.
It's generally not a good idea to add default values or fallbacks to get around potential crashes when it should be crashing.
waicool20
Got really annoyed I just had to read up some python for debugging haha, since its almost impossible to run sub switch configurations smoothly without a crash at this point.
So for some reason text_ref is not instance of str or Match causing an error that variable text is unreferenced, and since lines 450-457 in combat.py simply ignore the exception, the error is delegated to a seemingly unrelated part of the code (line 648 combat.py). With this change it reads the timer correctly without a problem. And also simply throwing an exception away is pretty bad, a stacktrace is always helpful for debugging.
And well since there are only 2 options, it might be better to default to Match anyways, even better is probably to add a text = "" assignment to the beginning of the function so that the check_timer function can loop until it reaches the fail limit even if the OCR fails.