Skip to content
This repository has been archived by the owner on Dec 10, 2017. It is now read-only.

Repair enhancements #270

Merged
merged 5 commits into from
Feb 7, 2017
Merged

Conversation

waicool20
Copy link
Contributor

@waicool20 waicool20 commented Jan 29, 2017

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:

[2017-01-29 02:38:40] At Home!
[2017-01-29 02:38:40] Are there returning expeditions to receive?
[2017-01-29 02:38:43] No, no fleets to receive!
[2017-01-29 02:38:44] Navigating to repair screen with 0 sidestep(s)!
[log] CLICK on L(186,438)@S(0)[0,0 1920x1080] (521 msec)
[2017-01-29 02:38:49] Got invalid timer ()... trying again!
[2017-01-29 02:38:51] Got valid timer (00:12:55)!
[2017-01-29 02:38:51] Got valid timer (00:09:28)!
[2017-01-29 02:38:54] Cannot repair; docks are full. Checking back at 2017-01-29 02:46:51!       <----- Sets wait time correctly  
[2017-01-29 02:38:58] Navigating to fleetcomp screen with 1 sidestep(s)!
[log] CLICK on L(118,395)@S(0)[0,0 1920x1080] (522 msec)
[log] CLICK on L(108,214)@S(0)[0,0 1920x1080] (522 msec)
[2017-01-29 02:39:11] No ships being repaired at the moment. Continuing sorties!      <-----Got reset
[2017-01-29 02:39:11] Attempting to switch out submarines!
[2017-01-29 02:39:11] Lets resupply fleets!
[2017-01-29 02:39:18] Navigating to resupply screen with 0 sidestep(s)!
[log] CLICK on L(88,292)@S(0)[0,0 1920x1080] (523 msec)
[log] CLICK on L(203,206)@S(0)[0,0 1920x1080] (522 msec)
[2017-01-29 02:39:24] Done resupplying!
[2017-01-29 02:39:24] Focus on KanColle!
[log] App.focus:  [803:Chromium]
[2017-01-29 02:39:39] Going home with 0 or less sidestep(s)!
[log] CLICK on L(157,354)@S(0)[0,0 1920x1080] (521 msec)

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.
@waicool20 waicool20 changed the title Checking next sortie times instead of using hardcoded 5 minutes Repair enhancements Jan 30, 2017
@waicool20
Copy link
Contributor Author

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

@mrmin123
Copy link
Owner

mrmin123 commented Feb 3, 2017

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.

@waicool20
Copy link
Contributor Author

waicool20 commented Feb 4, 2017

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.

Instead of after seeing there are no slots to repair. Script will loop
if there is a ship in fleet under repair, but there is still an empty
slot. Timer doesn't get set unless port is full
@mrmin123
Copy link
Owner

mrmin123 commented Feb 7, 2017

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.

@waicool20
Copy link
Contributor Author

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

@mrmin123
Copy link
Owner

mrmin123 commented Feb 7, 2017

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.

@waicool20
Copy link
Contributor Author

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

@mrmin123 mrmin123 merged commit 3f4aeff into mrmin123:master Feb 7, 2017
@mrmin123
Copy link
Owner

mrmin123 commented Feb 7, 2017

Regarding Python: you are entitled to your opinions.

Thanks for the PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants