mrmin123 / kancolle-auto

Kantai Collection (Kancolle) bot/automation tool - DEPERECATED - see kcauto-kai:
https://github.com/mrmin123/kcauto-kai
54 stars 22 forks source link

Added check in case no matches found #273

Closed waicool20 closed 7 years ago

waicool20 commented 7 years ago

Forgot to make these changes before you merged, moved the sort function into the next_sortie_time_set() function, and also added some code to prevent NoneType iteration exception if no ships in some damaged condition were not found.

mrmin123 commented 7 years ago

The PR has merge conflicts, I'm guessing primarily with the changes in next_sortie_time_set(). Can you merge master into your branch and update the PR?

mrmin123 commented 7 years ago

Actually, correct me if I'm wrong, but is the patch even necessary? There's a check for the image

if self.kc_region.exists(Pattern(image).similar(self.dmg_similarity)):

right before it which would keep that code block from executing if it wasn't found in the first place.

waicool20 commented 7 years ago

Well now that you've mentioned it, it does seem weird that it acts the way it does and very consistently, had 20+ crashes with the same exception overnight. So I'm guessing it's a bug with SikuliX OR, it's finding the image in the first check but the second findAll() isn't, but for what ever reason it's returning None, might be redundant but it stops the error.


[error] --- Traceback --- error source first
line: module ( function ) statement 
541: combat (  switch_sub )     for i in self.kc_region.findAll(Pattern(image).similar(self.dmg_similarity)):
524: combat (  go_repair )     if self.switch_sub():
458: main (  kancolle_auto_wrapper )     combat_item.go_repair()
524: main (  <module> )     kancolle_auto_wrapper()
[error] --- Traceback --- end --------------
mrmin123 commented 7 years ago

Hmm.. I haven't been able to reproduce so I'm going to hold off on merging this for now unless I get more reports of this causing issues. You might have to maintain that bit of code on your end. It's also entirely possible that with the most recent version of master the errors won't occur.

Also your PR still has merge conflicts. Please bring your branch up to date and resolve any conflicts before you create further PRs.

waicool20 commented 7 years ago

My bad for not updating the codebase, I might try testing it without the None check.

OTOH I removed the return statement at the bottom of switch_sub(), you're ending the function too early, it might not have been able to check the other dmg states if it couldn't find the first one

mrmin123 commented 7 years ago

That's a good catch! Upon review none of the returns in that for loop make sense. Going to to have to revise that logic entirely. Will be handled in #277

edit: refer to branch 277_switch_sub for testing new return logic

waicool20 commented 7 years ago

NoneType error seems to be gone now after using the latest commits, I guess I'll close this one

mrmin123 commented 7 years ago

Thanks for checking!