Leveling fixes

[CarlPoppa1]

• Moved runs to either already existing boss runs or already existing functions within the quest run
• Fixed other issues within leveling_act1.go to leveling_act5.go
• Moved option to use thawing/antidote potions to Duriel and Andariel main boss runs
• Added option to clear levels of Forgotten Tower in Countess run (we can't get to level 5 of the tower by running past mobs before we can teleport)
• Changed some run logic in act 1 leveling script (Den of Evil til 3, Cold Plains til 6, Stony Field til 9, Countess til 14 then Andariel)
• Fixed issue with traversing areas and discovering waypoints erroring if we have to return to town (needed for act 5)
• Added anvil UI position for classic (needed to open door to Duriel) and other UI positions for classic and resurrected (e.g skill list, merc list)

I've deliberately left out the changes I made to open doors more reliably. So this PR will only work smoothly once #697 has been merged. Both PRs will need to be tested together.
It also requires D2Go #91 to be merged in order to bind TP tome.

More changes to come...
Skill allocation
Stat allocation (with struct to allow for stats to be defined more than once)
Dodge
Autostart (don't need tome portal bound)
Option to set optimal settings

[Wamlad]

What do you mean by "Stat allocation (with struct to allow for stats to be defined more than once)"? Unsure what you mean by defined more than once. Is that to do with the autoequip?

Some potential fixes on your more changes to come list below. Would like to help out where I can.
Apologies for the wall of text. Can DM to take any discussion out of the PR if preferred.

Stat allocation and Skill allocation can use PR 674 #674 . It only changes leveling_tools.go so no impact on your changes. It also doesn't require any changes to the character scripts, so should be fine with your autoequip work? Assuming stat points will be amended in those scripts to meet strength requirements etc.

Autostart without TP bound, I've tested a solution to this (or a workaround at least). Just need some time to amend it in this PR.. If it's easier for you to add or can improve on the workaround:

• In each of the character leveling scripts, need to amend 'requireKeybindings' to remove 'skill.TomeOfTownPortal'
• In 'leveling_tools.go' remove the line 'skillsToBind = append(skillsToBind, skill.TomeOfTownPortal)' as this was a duplicate, it's already included in the character leveling scripts.
• This will effectively start the game without TPs, but the logic is already built in to buy Tome of TP and ID if it isn't found. So it will just farm the first area until it can. It will then bind the skill too.
• Bot settings should have return to town for HP and MP unticked. This isn't ideal, as they'll need to be switched back on later in the Koolo UI. Can that setting be overridden if char is less than level 3 or something? It's triggered in internal/bot/bot.go but I don't know if that setting can be overridden when the bot is running, outside of changing the actual config.yaml and clicking reload configs.
• Only other suggestion is to maybe change the chest opening from false to true for the Den of Evil since it's the first area. Might help get that gold sooner to buy Tomes.

Other suggestion: There is lots of running through Act 1, so when traversing to new areas or even clearing areas I have been repurposing the Andy script section to buy Stamina Potions at the start of these functions (instead of antidote pots). Helps with speed and survivability. Especially running to Countess, 4 pots seemed sufficient if not clearing monsters.

[Wamlad]

Cold Plains function needs a quick fix:

MoveToArea needs to be directly adjacent, so it can't run this straight from the Rogue Encampment. Whereas WayPoint will traverse all the way to it if the character doesn't have it yet.

[CarlPoppa1]

What do you mean by "Stat allocation (with struct to allow for stats to be defined more than once)"? Unsure what you mean by defined more than once. Is that to do with the autoequip?

Current implementation uses a map with a key of stat.ID to assign stat points. That means we can only apply any single stat in one consecutive action or actions. In reality, we want to assign a few strength points at level 3, a few at level 12 etc to make sure that we can equip better items as we move through the acts. I've moved it to a struct where you input your target points, once that target has been reached it moves on to the next entry in the struct:

func (s SorceressLeveling) StatPoints() []context.StatAllocation {

	// Define target totals (including base stats)
	targets := []context.StatAllocation{
		{Stat: stat.Vitality, Points: 50},  // Base 10 + 40
		{Stat: stat.Strength, Points: 25},  // Base 10 + 15
		{Stat: stat.Vitality, Points: 65},  // Previous 50 + 15
		{Stat: stat.Strength, Points: 47},  // Previous 25 + 22
		{Stat: stat.Vitality, Points: 999}, // Rest into vit
	}

	return targets
}

Autostart without TP bound, I've tested a solution to this (or a workaround at least). Just need some time to amend it in this PR.. If it's easier for you to add or can improve on the workaround:

* In each of the character leveling scripts, need to amend 'requireKeybindings' to remove 'skill.TomeOfTownPortal'

* In 'leveling_tools.go' remove the line 'skillsToBind = append(skillsToBind, skill.TomeOfTownPortal)' as this was a duplicate, it's already included in the character leveling scripts.

* This will effectively start the game without TPs, but the logic is already built in to buy Tome of TP and ID if it isn't found. So it will just farm the first area until it can. It will then bind the skill too.

Yes, I've done exactly the same, I just haven't pushed the commit yet.

* _Bot settings should have return to town for HP and MP unticked._ This isn't ideal, as they'll need to be switched back on later in the Koolo UI. Can that setting be overridden if char is less than level 3 or something? It's triggered in internal/bot/bot.go but I don't know if that setting can be overridden when the bot is running, outside of changing the actual config.yaml and clicking reload configs.

I've set it to be based on the amount of gold (in act 1 only but maybe it needs to be for all acts) - it get switched back on when we have enough gold and turned back off when we drop below a certain level.

* Only other suggestion is to maybe change the chest opening from false to true for the Den of Evil since it's the first area. Might help get that gold sooner to buy Tomes.

Good idea, I'll do this, thanks for reviewing.

[Wamlad]

Sounds like it's all getting there, appreciate the responses. Let me know if I can assist with anything, but happy to test/review when I can.

Re: the stats allocation, is that not what it already does?
Rather than being a list that is progresses through one by one, the only difference (in my mind) is that the current setup shows which level it's triggering the different thresholds. To me this is the exact same thing but more transparent and editable.

The existing one below is from sorceress_leveling_lightning.go. I would change this to get strength earlier, but didn't think it was worth making a PR until autoequip would take advantage of it. But it's very clear for anyone editing what levels these start triggering. So just saying it's something that probably doesn't need an overhaul, and has a working PR if that's something you can save your time on.

`statPoints := make(map[stat.ID]int)

if lvl.Value < 9 {
	statPoints[stat.Vitality] = 9999
} else if lvl.Value < 15 {
	statPoints[stat.Energy] = 45
	statPoints[stat.Strength] = 25
	statPoints[stat.Vitality] = 9999
} else {
	statPoints[stat.Energy] = 60
	statPoints[stat.Strength] = 50
	statPoints[stat.Vitality] = 9999
}`

[Antizerg]

Current script isn't clearing tower floors, which is not ideal for leveling.
I believe you are missing the countess.go update on this PR that you had intended.

The current countess.go on main does not have any action.ClearCurrentLevel.

But we also need to make it a configurable, like you intended so a normal countess run isn't impacted by this.

I believe if you add your local countess.go to this PR, it will fix the issue.

[Antizerg]

Balance Update: Current script has us going to Countess at lvl 9. This is resulting in a lot of chickens. I recommend hitting countess at level 12. For a sorc, this allows for fireball, which is a decent power creep and allows for clearing countess much easier. Right now at lvl 9-10, I am chickening 100% of the time. If I wait until lvl 12, that lowers chicken rate significantly.

Clearing Stoney until 12 just requires one parameter update in your code. We might be able to maximize this and just clear BlackMarsh until level 12, not sure what would be faster. I can test this if you want, but for now I am just going to settle with clearing stoney until 12.

[Wamlad]

@Antizerg try running leveling as lightning. Firebolt with no AOE will be far less effective I imagine.
Also was your chickening happening while clearing each floor? Clearing each floor problably helps solve this too.
The actual countess kill will get a lot of chickens early on. So could always have two functions, clear levels 1-4 until level 12, clear all 5 after level 12? The tower levels are narrow and lots of elite/champ packs so its far quicker for leveling in my opinion.

@CarlPoppa1 I know you simplified the runs to match the existing boss runs but I think leveling will sometimes call for a different approach. i.e. Act 1 normal since we have to walk.
I've just finished implementing the ClearThroughPath actionfrom the Diablo script and it's working well for leveling_act1.go.
I suggest adding this instead of the existing Countess boss run, for normal at least.
Using the ClearThroughPath action could also make navigating Barracks, Jail, Catacombs much easier opening doors without being blocked by mobs. If we clear along the way, we won't be far off level 18 & teleport for Act 2.

func (a Leveling) countessClearPath() error {
	// Travel to boss level
	err := action.WayPoint(area.BlackMarsh)
	if err != nil {
		return err
	}

	// Move to these areas directly
	action.MoveToArea(area.ForgottenTower)
	action.MoveToArea(area.TowerCellarLevel1)

	// Destination areas to clear towards
	areas := []area.ID{
		area.TowerCellarLevel2,
		area.TowerCellarLevel3,
		area.TowerCellarLevel4,
		area.TowerCellarLevel5, 
	}

	for _, ar := range areas {
		// Get ingame adjacent areas so we can read the exit coordinates, check it matches the next level from script
		adjacentLevels := a.ctx.Data.AdjacentLevels
		nextExitArea := adjacentLevels[0]
		for _, adj := range adjacentLevels {
			if adj.Area == ar {
				nextExitArea = adj
			}
		}
		// Get the exit coordinates (position) and clear towards it
		nextExitPosition := nextExitArea.Position
		action.ClearThroughPath(nextExitPosition, 15, data.MonsterAnyFilter())
		err = action.MoveToArea(ar) // MoveToArea still needed to click exit
		if err != nil {
			return err
		}

	}
	// Countess section from the boss script added, plus ClearThroughPath
	// Try to move around Countess area
	action.MoveTo(func() (data.Position, bool) {
		if areaData, ok := context.Get().GameReader.GetData().Areas[area.TowerCellarLevel5]; ok {
			for _, o := range areaData.Objects {
				if o.Name == object.GoodChest {
					action.ClearThroughPath(o.Position, 30, data.MonsterAnyFilter())
					return o.Position, true
				} // Countess Chest position from 1.13c fetch
			}
		}

		// Try to teleport over Countess in case we are not able to find the chest position, a bit more risky
		if countess, found := a.ctx.Data.Monsters.FindOne(npc.DarkStalker, data.MonsterTypeSuperUnique); found {
			action.ClearThroughPath(countess.Position, 30, data.MonsterAnyFilter())
			return countess.Position, true
		}

		return data.Position{}, false
	})

	// Kill Countess
	return a.ctx.Char.KillCountess()
}

[Antizerg]

The act 1 go script isn't saving cain properly. It will go into stoney after the scroll and immediately leave. It also doesn't right click on the scroll to activate the shrines.
The logs don't show anything as well, but attached is the log.
Supervisor-log-NewBlizzy-2025-02-28-15-45-42.txt

[Wamlad]

Some of the quest scripts no longer work, so den, cain etc. can be skipped or feel free to fix them too :)

Andariel quest will never get completed by the way. This script triggers the main boss farming run instead of the leveling function. It needs that leveling function to go talk to Warriv and get to Act 2.

#713
I recommend using this draft PR with walkable logic. Solves door issues and getting stuck on mobs when no teleport. Also helps with exp. ClearThroughPath action not required as this kind of builds it in (when walking only).

#718
I lodged this issue, but checking you ran into this too with Rocky Waste?
When first starting act 2 it can't traverse Rocky Waste so constantly quits outside of town. It basically needs DryHills waypoint manually or the bot will never get it.

[Antizerg]

Based on latest files.
Act 2 Bugs:

1. It seems to never acknowledge summoner is completed and will keep running it over and over
2. It doesn't create staff and put it in oriface

Act 3 Bug:

1. It collects all of the organs, makes the will, breaks the altair, but then goes back to spider forest to start collecting organs again. It never goes into durance to grab WP or go after mephisto

[Antizerg]

Act2 bug:
It has happened a few times now, but there is a scenario where both "The Summoner" quest won't be completed, but "The Seven Tombs" is active, showing the final tomb symbol. The bot will just join the game and immediately exit it as soon as it determines the quest log.

I think the only way to solve this is to ensure the summoner quest is fully completed before running line 56:
if !a.ctx.Data.Quests[quest.Act2TheSevenTombs].HasStatus(quest.StatusQuestNotStarted) {

[CarlPoppa1]

Act2 bug: It has happened a few times now, but there is a scenario where both "The Summoner" quest won't be completed, but "The Seven Tombs" is active, showing the final tomb symbol. The bot will just join the game and immediately exit it as soon as it determines the quest log.

I think the only way to solve this is to ensure the summoner quest is fully completed before running line 56: if !a.ctx.Data.Quests[quest.Act2TheSevenTombs].HasStatus(quest.StatusQuestNotStarted) {

You're right. I've fixed it differently though. I've just moved the Duriel check to after the Summoner check. So it'll keep running Summoner until Summoner is killed.

[Fujiyama69]

I've not tried this branch yet, but I appreciate your effort.
In my own testing and slow pace of fixing the leveling I did come across a problem where it killed the summoner but TP'd, and exited the run before interacting with the tome and using the waypoint.

Here's my fix for that specific case in case it is relevant and helpful:

	// We might have killed the summoner already, but we need to finish the quest to get the waypoint
	if a.ctx.Data.Quests[quest.Act2TheSummoner].Completed() &&
		!a.ctx.Data.Quests[quest.Act2ArcaneSanctuary].Completed() {

		a.ctx.Logger.Debug("Summoner already killed, but quest Act 2 Arcane Sanctuary not completed")
		waypointErr := action.WayPoint(area.ArcaneSanctuary)
		if waypointErr != nil {
			a.ctx.Logger.Error("Error waypointing to Arcane Sanctuary", "error", waypointErr)
			return waypointErr
		}
	} else {
		// Start Summoner run to find and kill Summoner
		err = NewSummoner().Run()
		if err != nil {
			a.ctx.Logger.Error("Error running Summoner", "error", err)
			return err
		}
	}

	tome, found := a.ctx.Data.Objects.FindOne(object.YetAnotherTome)
	if !found {
		a.ctx.Logger.Error("Yet Another Tome not found")
		return fmt.Errorf("yet another tome not found")
	}

	// Try to use the portal and discover the waypoint
	err = action.InteractObject(tome, func() bool {
		_, found := a.ctx.Data.Objects.FindOne(object.PermanentTownPortal)
		return found
	})
	if err != nil {
		a.ctx.Logger.Error("Error interacting with Yet Another Tome", "error", err)
		return err
	}

	portal, found := a.ctx.Data.Objects.FindOne(object.PermanentTownPortal)
	if !found {
		a.ctx.Logger.Error("Permanent Town Portal not found")
		return fmt.Errorf("permanent town portal not found")
	}

	err = action.InteractObject(portal, func() bool {
		return a.ctx.Data.PlayerUnit.Area == area.CanyonOfTheMagi && a.ctx.Data.AreaData.IsInside(a.ctx.Data.PlayerUnit.Position)
	})
	if err != nil {
		a.ctx.Logger.Error("Error interacting with Permanent Town Portal", "error", err)
		return err
	}

Another issue I encountered was in the Harem/Palace cellar area were there can be multiple transitions to the same area. The bot had problems interacting with the area transition in the cases where there were > 1.
I solved this by sorting by distance in InteractEntrance:

		// Find all transitions to the target area and sort by distance
		type transitionInfo struct {
			level    data.Level
			distance int
		}
		var transitions []transitionInfo

		for _, l := range ctx.Data.AdjacentLevels {
			if l.Area == area {
				distance := ctx.PathFinder.DistanceFromMe(l.Position)
				transitions = append(transitions, transitionInfo{
					level:    l,
					distance: distance,
				})
			}
		}

		// If no transitions found, continue
		if len(transitions) == 0 {
			continue
		}

		// Sort transitions by distance (closest first)
		sort.Slice(transitions, func(i, j int) bool {
			return transitions[i].distance < transitions[j].distance
		})

		// Process the closest transition
		l := transitions[0].level
		distance := transitions[0].distance
		
		.... Existing code

		// If there are more transitions to try, continue to the next iteration
		if len(transitions) > 1 {
			// Try the next closest entrance
			continue
		}
		
		// Log that we found a transition but it's not an entrance
		ctx.Logger.Debug(fmt.Sprintf("Found transition to %s but it's not an entrance", area.Area().Name))

		return fmt.Errorf("area %s [%d] has transitions but none are entrances", area.Area().Name, area)

[Timmynat0r]

Found Issue when using ensureKeyBinding: true and legacy mode, bot is clicking on belt slot1 and gets stuck with potion in hand

[CarlPoppa1]

Found Issue when using ensureKeyBinding: true and legacy mode, bot is clicking on belt slot1 and gets stuck with potion in hand

Yeah I hadn't pushed my ensureKeyBinding changes. Please can you try again when you get chance? It should be fixed in legacy and normal graphics now. Thanks for testing.

[Timmynat0r]

Found Issue when using ensureKeyBinding: true and legacy mode, bot is clicking on belt slot1 and gets stuck with potion in hand

Yeah I hadn't pushed my ensureKeyBinding changes. Please can you try again when you get chance? It should be fixed in legacy and normal graphics now. Thanks for testing.

this fixed the legacy issue thx

I also posted some related issues on #699

[Timmynat0r]

Another small suggestion i have, would be the following:

Since you changed the logics for Andariel and Duriel to fit your leveling logic, its hard to merge this pr into other stuff, because you also changed some lines on the original logic. Like in duriel.go:

action.Buff() before Bossfight is now only done if usethawing is active
interacting with duriel entrance is also within the same function, maybe more

They way i fixed those merge issues was to make a copy of duriel.go, named it duriel_leveling.go and change the calls you now use twice to ....Leveling, in the long run this is maybe not the best idea, because we dont want to blowup the filescount thats getting used, but for PR testing i find this quite nice because we do not mess up the main logics that way and dont get major merge conflicts.

And its also nice for debugging and readability, while testing.

You can see it in action in my Custom Branch:
https://github.com/Timmynat0r/koolo/tree/Leveling%2BCompanion

Maybe you want to implement it like that for the moment, if not just ignore my wall of text :)

[jdsmi]

For the stat and skill assignment I made a similar PR (#766) with a similar approach though I didn't modify the original return type of the stat function like you did.

Overall looks SUPER good!

[jdsmi]

I think there's a slight false sense of success here. Since this value depends on being refreshed from the update loops it's possible this value won't be accurate. I think what makes it seem like it works is the delay due to clicking the button. However if the anything happens with the update this could lead to an incorrect result.

It might be easier just to rely on remainingPoints. The worst thing that could happen is a failed click and a point doesn't get assigned, this would just be reconciled next check for available points and the unspent point would be properly allocated.

[CarlPoppa1]

Yeah, I'll revert these commits and go with your implementation (although there are still conflicting PRs aiming to do the same thing)

[jdsmi]

what's wrong with ctx.Data.PlayerUnit.Class?

[CarlPoppa1]

Nothing, dunno what I was thinking. I'll change it

[jdsmi]

This would be made unnecessary with #713 finished and completed.

[CarlPoppa1]

Agreed. I've made progress with finishing that PR, just testing it on leveling runs and refining it

[jdsmi]

Curious why removing the trist runs after 6? It seems like this flow is also a bit more complex than it needs to be. So was the original flow but I did like the later level runs.

A combo of both would be great:

1. Den when < 3 or den not completed
2. Cold Plains < 6
3. Cain Quest until complete
4. Trist until 15 or nightmare though we're getting a bit bloated trying to make this a once size fits all, might be better to just make a leveling nightmare run or something.
5. Countess until 18
6. Andy until someone shuts off the bot and increases difficulty

[CarlPoppa1]

In my runs I was repeatedly chickening at Treehead Woodfist at the Inifuss Tree. I had much more success going straight to Tower and leveling there.
Countess to 18 is very slow. I think we should always aim to be within 5 levels of each area level, which means we should stop countess at level 12.

I don't understand step 6 in your list but I'm not a fan of a leveling script that needs manual intervention

[jdsmi]

this seems like it should be an option on the andy run like NewAndarial(continueToAct2 bool) or something

[jdsmi]

if this is now a setting why override it?

[CarlPoppa1]

Because (in normal difficulty as a sorc) it's almost impossible to kill Duriel without being either twinked and/or over leveled because we can't kite like a player can. In my experience it's only remotely possible if we use thawing pots

[jdsmi]

probably out of scope here and not your code but why does it matter if this is a leveling character to pick up the staff?

Shouldn't any run that needs the staff pick it up?

[CarlPoppa1]

Yeah I guess but I think leveling is the only run that needs the staff, but I guess you're right that no other run will ever go to maggot lair so it's probably an unnecessary condition

[jdsmi]

I assume this is due to firebolt being a +skill on an item?

If so is there a way to make this reusable so it can be used in the future by other functions? I'm thinking lower res wands or teleport staff/amulet

[CarlPoppa1]

Yeah, I haven't bothered properly accounting for +skills or charges on the key binding script but you're right it would be helpful to do that and to move the key binding functions somewhere more generic to be used for non-leveling. It's not a priority for me at the moment though

[jdsmi]

I had a similar thought change by making it just an array of stat.ID returned, it looks more verbose than your implementation but has the same function.

Another feature that can be added following your approach is the ability to dynamically set certain stat levels. For example the amount of dexterity required to reach a certain block rate or amount of strength to wear all currently equipped gear (after a respec). Less important now since there's no autogearing but something that would be useful in the future.

[jdsmi]

I think straight points to spend is more clear here than target totals. This of course would mean knowing the base stats of a character class but that's not hard to put together.

[CarlPoppa1]

I think your implementation is better so I'll reverse my commits relating to skill and stat point allocation