Closed waicool20 closed 7 years ago
When orel cruising and enabling sub switching, Kancolle Auto uses the last repaired ship repair time as the next sortie schedule even when there might possibly another sub with a shorter repair time. So time is wasted even though it could have used switched into the sub that repaired quicker. I've added a few lines to set the next sortie time to the quickest repair if sub switching is enabled.
Also added crashes folder to gitignore
Sorry I'm getting so long to getting this PR in. Been pretty busy recently. I'll hopefully be able to take a look over the weekend.
Take your time. Meanwhile I've removed the return statement in the last commit so it attempts to switch subs even if port is full. The loop was caused by 1. no subs were repairing because ports were full 2. you only checked for a repair pattern when in fleetcomp screen. So when it went to fleetcomp screen it just returned True because it didn't find any ships in repair. I've made it scan all the possible damage states, and switch those subs out. A slight improvement? might be to skip any damage state above the repair limit, but its always nice to switch and repair subs any time even if its only scratch damage to recover morale. With that said, the next step is to try repairing damaged subs before every sortie, the script doesn't track the switched out subs that are damaged but not repairing.
A couple of things:
I have some reservations about calling go_repair
method directly from within the go_sortie
method, but it does kinda get the job done... One edge-case scenario that might cause significant delays: If on boot there is a ship that is ALMOST done repairing, so it fails sortie, then goes to repair, but in that time the repair finishes, but there are other unrelated ships being repaired, the next_sortie_time_set
gets set to whenever a ship on that list is done repairing.
I don't think that last commit is necessary. Moving the next_sortie_time_set()
call from the second except
to the first try
doesn't do anything other than make it get called every time there's a repair_timer_alt.png
match, which is unnecessary. I would instead move it back, remove the repair_timers.sort()
in the first try
block, and move it into the next_sortie_time_set
method directly:
def next_sortie_time_set(self, hours=-1, minutes=-1, flex=0, override=False):
if hours == -1 and minutes == -1:
if len(self.next_sortie_time) > 0:
self.repair_timers.sort()
self.next_sortie_time = self.repair_timers[0]
else:
self.next_sortie_time = datetime.datetime.now()
This would actually solve a potential issue with your additional 'Set fatest repair time (which might be a sub)' block, which does not sort the repair_timers
list which might have been altered during the actual repair loop.
Your idea of looping and going through all damaged ships is fine, but it's incredibly heavy-handed and arguably undesirable for anything other than orel cruising. I'd say add a conditional that defines scan_list
based on the map kancolle-auto is set to sortie to.
Please remember kancolle-auto isn't designed to only do orel cruising, so outlier behavior for other scenarios and use cases must be accounted for.
I still think instead of map based scan_list, it's safe to just use the submarine switch conditional to determine it.
And I believe the next_sortie_time_set()
should be in the first try block. The script goes into a loop where it tries to sortie but cannot due to a ship in the sortieing fleet being repaired AND there is an empty repair slot. It doesn't need to be the submarine in the fleet, it could be any arbitrary ship. If not, it sets it correctly and works as intended
I'll test moving the self.repair_timers.sort()
to the next_sortie_time_set()
function and then commit, though I'm not sure about the code block you gave me, afaik a len()
on a datetime
object doesn't make sense and should give an error.
I've been testing out these modifications for the last few days, and it's been a much better experience in terms of reliability and consistency overall.
And on a side note, just wondering why you decided to go the Jython route instead of just using Java, my current experience right now trying to patch stuff here and there is horrible with the dynamic typing of python lol, even with the wonderful autocomplete of IntelliJ+Python plugin
It wouldn't work based off of the submarine switch flag because the user might be using it on something like 3-2A or 5-4A where a bait submarine is used. You wouldn't want to look through the other damaged ships in this context.
I guess that looping behavior is the result of calling go_repair
directly from the go_sortie
method, instead of waiting for it to naturally finish. I guess it's necessary to move it to the first try
block then.
It should be len(self.repair_timers)
, sorry.
It's great that your modifications are more consistent, but if you're not already taking it into account when you're putting the logic together, you should be testing it for all cases where the combat and repair modules might be used, not just Orel which seems to be what your modifications seem to mostly target. I personally don't even run Orel anymore so much of these improvements, while I am thankful for them, do not effect me. On the contrary, it actually makes things worse for my use cases (see para 1).
I went with Python because I've been coding in it professionally for many many years. I have no issues with it (I would argue that it's my favorite language) and I don't even use an IDE with autocomplete :) Although it certainly helps that I wrote 99% of the codebase.
Well actually I've been testing it for orel cruising, 3-2 and 1-5 leveling and well sure it might be checking other ships status too, at most it just wastes a little bit of extra time on that case since the script itself checks if the ship is a sub or not...safely ignoring other ships.
If on boot there is a ship that is ALMOST done repairing, so it fails sortie, then goes to repair, but in that time the repair finishes, but there are other unrelated ships being repaired, the next_sortie_time_set gets set to whenever a ship on that list is done repairing.
As for that one case, I'm thinking after checking the repair screen, it switches to the fleet comp screen to double check first fleet condition. Again might use a few more seconds but is much more fool-proof. And well since it's almost finished anyways, might as well wait and let it run its course before running the script.
I guess that looping behavior is the result of calling go_repair directly from the go_sortie method
The looping existed before the patches, so I don't think that's the case.
OTOH python is pure evil in terms of collaboration of then, I have to keep on jumping to declarations/following the script and stuff just to find what type a variable is, can't work on a section of code in peace
Regarding Python: you are entitled to your opinions.
Thanks for the PR.
Just some more enhancements after studying the code of kancolle auto
Also added return statement if docks are full, since the code below shouldn't be executing anyways. Without it the next sortie time tends to reset to 0 if sub switching is enabled even if their are still subs to be repaired. Causing a loop.
It starts the next sortie immediately, sees that a sub needs repair, goes to repair screen to find out that there are repairs still goes on, then goes trying to sub switch again which resets the wait time. Rinse and repeat.
Example log: