Fix Ganking Code

[Nostrademous]

After the massive changes I made in last check-in the gank code can go haywire

[Keithenneu]

I finally found out hat causes bs to screw up junlging and ganking:

Once he is lvl6, he adds action_ganking to the stack. If he can't find a target the decision tree will fall through to jungling. Since jungling is still on the stack, it' wont be put back at the top of the stack. So he is jungling with the wrong action assigned, which screws some things up.

Might not be directly related to this issue, as this problem existed before. #68

[Nostrademous]

you mean action_ganking, but that's okay. the mess up is in the fighting code and when it clears the target... I think I have it fixed, testing.

[Keithenneu]

oh right, fixed it. This happens even without any fighting though. He's farming a camp, gets 6, can't find a target, and continues jungling with the wrong action. (Which eventually blows up getPrevAction(), which I need to restart the jungling state machine)

[Nostrademous]

Oh.. I didn't realize you need it proper for resetting jungling state machine. I was just handling it

[Nostrademous]

will have to think about it

[Nostrademous]

this is taking forever to fix...

[Keithenneu]

How can I help?

[Nostrademous]

Not much to do... just a time sink. I just realized the issue "most likely" is in the fact that I was using a lot of checks like this:

if utils.NotNilOrDead(target) then Do Stuff - Like go to target else Clear Action and Hero Variables end

The issue is that a target can be "dead" but not "nil" - still a valid handle to a target (not nil), just "dead". This was causing some funky loops since I wasn't resetting things properly after gank was completed

[Nostrademous]

The other part is the following (and I'm not sure how you intended it to work).

We find a valid gank target.. we get to them... we transition to ACTION_FIGHT.. ACTION_FIGHT estimates that we cannot kill them without risks to ourselves... so we rupture them, and run away....

I just added code to make sure that we can kill a target before we start ganking them. Checking how it works.

[Keithenneu]

I already calculate chances based on distance to towers, enemy health and number of heroes on each side. We should probably just use the same functions for these estimations.

[Nostrademous]

I don't think you calculated a chance of getting a kill. You calculated a ranking of easiest-to-hardest to gank based on towers, health, numbers of heroes on each side (kind of for enemies here... b/c if you don't see an enemy you don't have their location so you only account for visible enemies... you could use "GetLastSeenLocation()" for those you can't see and providing "GetTimeSinceLastSeen()" isn't too long use that). But in the end you get a sorted ranking... but THAT DOES NOT guarantee you can get a kill, it just guarantees you are going after the easiest target.

You have to use GetEstimatedDamageToTarget() to try and see if you can kill it.

I will push what I have, still not working but I need to disappear for about 2.5 hrs. Feel free to try to solve this.

[Nostrademous]

pushed changes... also, you can try to use the stuff in enemy_data rather than local enemies = GetUnitList(UNIT_LIST_ENEMY_HEROES) -- check all enemies

Might be faster... but not sure

[Keithenneu]

What I wrote was an first attempt, nothing more. This way we can sort out issues, such as the one we're facing right now. Finding the easiest gank target, and the easiest kill target, should be highly correlated as well. I didn't have time to update my code to use the latest enemy code yet.

[Keithenneu]

If we are ganking, and the enemy get's tp support or something we should still be able to retreat and go back to jungling (or laning etc). A better estimation will make this happen less frequently. But it will not repair broken action transitions by any means.

[Nostrademous]

I am not sure that the action transitions are broken. It might seem that way but that is because we remove an action when we transition from Gank to fight with out doing a return out of the main Think function so the transition is not printed to console.

[Nostrademous]

back to working on this... I decided I will prevent the transitions removal from external actions... to simplify life

[Nostrademous]

I think i fixed it in commit e9a43c30c401876d37cdf9b6daa9ccaf9271e685 can't test as I'm dead tired and it's 1:30am and my kids will be up at 6:30am.

[Keithenneu]

I still got some "Gank target dead" spam, after which my game crashed without any errors. I pushed a PR,which should improve BS ganking slightly. It won't touch any ganking logic thouh. Btw, you had a typo in ganking_generic getfenv(). It's in the PR as well.

[Nostrademous]

btw... the reason this took so long is b/c the issue was a mis-spelled variable.

In the decision_tree:Think() I was doing if (self:GetAction() == ACTION_GANK or Determine_ShouldGank(bot))

but ACTION_GANK didn't exist... it was ACTION_GANKING

[Keithenneu]

FML. I want back my type checking languages. So ganking is all good now?

[Nostrademous]

Okay, testing on the latest push has "ganking fixed"; there is a bug with "resume jungling" though if he took tower damage during gank. It's a race condition seems like... between bloodseekers' DoRetreat with Tower reason and OnResume(). One wants to escape, other wants to go back to camp

[Keithenneu]

ok, I will look into this, thanks.

[Nostrademous]

updated my comment above

[Nostrademous]

It's interesting b/c it is a coincidence that he went into ACTION_GANKING, killed the target, tower shot projectile, he transitioned back to ACTION_JUNGLE, projectile from tower hit him... and he is confused b/c he is in JUNGLING mode going back to camp per OnResume() but got hit by a tower (and there is no towers in jungle)

[Nostrademous]

I'm adding a check to DoRetreat() for Tower reason to check if either we arrived at our location spot OR we haven't been hit by a tower for 3 seconds... if either true, stop retreating.

Should fix that case and make our retreat code better

[Nostrademous]

So tests show the GANKING transitions works fine... the "OnResume()" needs some review of how it interplays with things. I assume it's in your hands.

[Keithenneu]

This basically happens when DoJungle() is hit while the bot is in a mode other than ACTION_JUNGLING. I'll have closer a look at it tomorrow.

[Nostrademous]

You are right, but I'm surprised that happens. That means a higher-level ACTION is falling-through (return false somewhere) to evaluate DoJungle

[Nostrademous]

Any update here?

[Keithenneu]

Not yet. I'm busy with deadlines and exams. So things will go slowly on my end. I'll try to analyze the problem today. But don't expect it do be fixed before the weekend.

[Nostrademous]

No prob. I looked at it and thought about it. Pushed what I think will solve the issue by removing the need for OnResume() at all. Basically I now check if we are within 200 units of the camp before we set is as cleared to eliminate the problem we saw earlier where if BS was Ganking and then transitioned back to Jungling but ran into enemy wave creeps he could mistaken it for clearing the camp.

[Keithenneu]

This will introduce some inefficient behaviour for ranged junglers though. We should probably check if we have vision on the spawn point of the camp (Not necessarily from the hero though). But I think it's allright for now.