-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: updates to blizzard sorc attack loop #707
base: main
Are you sure you want to change the base?
Conversation
…onst; optimization of attackloop to minimize wasted iterations
useMerc: true | ||
stashToShared: false | ||
useTeleport: true # If set to false, bot will not use teleport skill and will walk to the destination | ||
|
||
sorceress: # generic sorceress configurations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
common configuration that can be leveraged via s.Data.CharacterCfg.Character.Sorceress.StaticFieldMinDist
type BlizzardSorceress struct { | ||
BaseCharacter | ||
} | ||
|
||
func (s BlizzardSorceress) attackConfig() map[string]int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed there were issues in the builds with accidental references across the characters which might have unintended results. I thought this might be a better pattern to make it more clear which settings are relevant for each character. Is there a better way?
@@ -53,6 +58,46 @@ func (s BlizzardSorceress) CheckKeyBindings() []skill.ID { | |||
return missingKeybindings | |||
} | |||
|
|||
func (s BlizzardSorceress) killMonsterWithStatic(bossID npc.ID, monsterType data.MonsterType) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mostly stolen from the nova sorceress
// Pull target threshold from config based on difficulty | ||
var targetThreshold int | ||
switch s.Data.CharacterCfg.Game.Difficulty { | ||
case difficulty.Normal: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values have a bit of a buffer over the bare minimum for the difficulty to minimize wasted casts
@@ -77,7 +128,8 @@ func (s BlizzardSorceress) KillMonsterSequence( | |||
return nil | |||
} | |||
|
|||
if completedAttackLoops >= sorceressMaxAttacksLoop { | |||
if completedAttackLoops >= s.attackConfig()["maxAttacksLoop"] { | |||
s.Logger.Error("Exceeded MaxAttacksLoop", slog.String("completedAttackLoops", fmt.Sprintf("%v", completedAttackLoops))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed a few people in Help
noting that their character is bailing early but not knowing why. I think many of them are from hitting this loop in their respective character. This log message should help identify those cases better.
@@ -86,39 +138,34 @@ func (s BlizzardSorceress) KillMonsterSequence( | |||
s.Logger.Info("Monster not found", slog.String("monster", fmt.Sprintf("%v", monster))) | |||
return nil | |||
} | |||
if monster.Stats[stat.Life] <= 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This avoids unnecessary attackloop iterations for bosses with an extended death animation like andariel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I've been testing with this after every attack in KillMonsterSequence, seems to work:
if s.Data.PlayerUnit.Mode == mode.CastingSkill {
attackSuccess = true
}
Then at the bottom:
if attackSuccess {
s.Logger.Debug("Completed attack loop", slog.Int("completedAttackLoops", completedAttackLoops))
completedAttackLoops++
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant using mode dead and death . Dead is when it just died, death is when its extremely dead ( after animation) or i inverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I meant to reply to the comment above about character bailing early, sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still have to check wich of the 2 mode is set after the animation otherwize it will still exit game early and have no real benefit from checking life stat
one of the mode tells its dead so we can stop attack sequence, the second one tells we can return and go on with next run( drops should be on ground/animation finished)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh gotcha i missed that distinction. i'll think about how I can implement that, i don't think it fits with how I set up the methods at this time, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also keep in mind koolo is able to read itemlocation dropping . Both location ground and dropping are merged together. This means it reads item before they are visible on ground. Could explain early exit symptoms if nothing match .nip rule . We dont have to see them on ground to evaluate if it should pickup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about what you said and I'm not sure the distinction matters for the attack loop. Either the monster isn't dead and we should kill it, or it's dead and we should do something else.
Can you give some more background on the problem you're describing? Should there be a short wait added in the run definition or in the KillMephisto definition?
return nil | ||
} | ||
|
||
for s.Data.PlayerUnit.States.HasState(state.Cooldown) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switched this to cast our leftskill until we're out of cooldown instead of just a single time. This should better leverage our activity during blizzard cooldown without resulting in a new attack loop.
I've pulled it ahead of the blizzard self-cast because otherwise we were usually hitting the self-cast block while blizzard was on cooldown.
New behavior should be:
while on cooldown:
cast leftskill
if timeToCastSelfBlizzard:
cast selfBlizzard
else
cast regularBlizzard:
previousUnitID = int(id) | ||
} | ||
} | ||
|
||
func (s BlizzardSorceress) killMonster(npc npc.ID, t data.MonsterType) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function was no longer being called
The blizzard logic is busted in this, that's what I get for doing a refactor late and night and pushing without testing. I've fixed it so it actually casts offensive blizzards again but I'm still seeing attack loops where nothing is performed. e: I think i've resolved this |
I ended up pulling out the logic for 'is this monster dead?' into a separate function, i'll see if elobo91's suggestion can make it simpler. |
9fac835
to
af33116
Compare
I had accidentally polluted my commits in #695 with bad line endings, this is a cleaned PR with just the blizzard sorc changes
This PR attempts:
I also included the switch from
sorceress
toblizzardsorceress
that was suggested in #691