LetsTimeIt / DungeonTools

Fork of Nnoga's Mythic Dungeon Tools addon to restore mob data and improve upon the base code!
GNU General Public License v2.0
281 stars 74 forks source link

Load correct sub-zone in instances? #21

Closed jarxjarx closed 3 years ago

jarxjarx commented 3 years ago

Not added with #20, seems like sub-zones are tied in deeper, so didn't see a simple fix like for main zone. Biggest QoL was for sure the instance.

CalistusK commented 3 years ago

I hacked a dirty solution together as follows (tested and working) in my personal fork of the addon. The main idea I had was to associate the sublevel with the zone ID, then set the sublevel in db.presets so that it would update along with loading the correct dungeon map.

I don't think my code is elegant, thus no pull request, but hopefully this helps.

    [1677] = {[1]=29,[2]=4}, -- de other side ardenweald
    [1678] = {[1]=29,[2]=2}, -- de other side mechagon
    [1679] = {[1]=29,[2]=3}, -- de other side zul'gurub
    [1680] = {[1]=29,[2]=1}, -- de other side
    [1663] = {[1]=30,[2]=1}, -- halls of atonement
    [1664] = {[1]=30,[2]=2}, -- halls of atonement nave of pain
    [1665] = {[1]=30,[2]=3}, -- halls of atonement sanctuary of souls
    [1669] = {[1]=31,[2]=1}, -- mists of tirna scithe
    [1674] = {[1]=32,[2]=1}, -- plaguefall
    [1697] = {[1]=32,[2]=2}, -- plaguefall festering sanctum
    [1675] = {[1]=33,[2]=1}, -- sanguine depths depths of despair
    [1676] = {[1]=33,[2]=2}, -- sanguine depths amphitheater of sorrow
    [1692] = {[1]=34,[2]=1}, -- spires of ascension honors ascent
    [1693] = {[1]=34,[2]=2}, -- spires of ascension gardens of repose
    [1694] = {[1]=34,[2]=3}, -- spires of ascension font of fealty
    [1695] = {[1]=34,[2]=4}, -- spires of ascension seat of the archon
    [1666] = {[1]=35,[2]=1}, -- necrotic wake
    [1667] = {[1]=35,[2]=2}, -- necrotic wake stitchwerks
    [1668] = {[1]=35,[2]=3}, -- necrotic wake zolramus
    [1683] = {[1]=36,[2]=1}, -- theater of pain
    [1684] = {[1]=36,[2]=2}, -- theater of pain chambers of conquest
    [1685] = {[1]=36,[2]=3}, -- theater of pain altars of agony
    [1686] = {[1]=36,[2]=4}, -- theater of pain upper barrow of carnage
    [1687] = {[1]=36,[2]=5} -- theater of pain lower barrow of carnage
}
local lastUpdatedDungeonIdx
function MDT:CheckCurrentZone(init)
    local zoneId = C_Map.GetBestMapForUnit("player")
    local dungeonIdx = MDT.zoneIdToDungeonIdx[zoneId]
    if dungeonIdx and ((not lastUpdatedDungeonIdx or dungeonIdx ~= lastUpdatedDungeonIdx) or
    db.presets[db.currentDungeonIdx][db.currentPreset[db.currentDungeonIdx]].value.currentSublevel ~= dungeonSubLvl) then
        lastUpdatedDungeonIdx = dungeonIdx[1]
        dungeonSubLvl = MDT.zoneIdToDungeonIdx[zoneId][2]
        db.presets[db.currentDungeonIdx][db.currentPreset[db.currentDungeonIdx]].value.currentSublevel = dungeonSubLvl
        MDT:UpdateToDungeon(dungeonIdx[1],nil,init)
    end
end
srenauld commented 3 years ago

There's no way to make it much nicer, to be honest. Keeping the map ID as key is the right call as this guarantees O(1) lookups rather than having to go through the entire list; probably the only change would be to actually name the properties (rather than relying on somebody knowing that the first entry in the sub-map is the dungeon, and second is the floor), and build a type check + fallback in case somebody ever decides to open MDT while in a BfA instance.

srenauld commented 3 years ago

@CalistusK I went for a slightly more radical refactor. Can you give it a whirl on https://github.com/LetsTimeIt/DungeonTools/tree/subzone-update , see what you think and if there's anything you'd change?

The changes:

I gave it a brief test in ToP. Having a floor selected while editing a route does not change your floor outside the instance; the moment you're in it, if you open MDT, it'll default you to the floor you're currently on.

CalistusK commented 3 years ago

Looks good to me! Much easier to look at than my version. 😄 I think we can use a default value for sublevel in UpdateToDungeon so we don't have to pass it in every time (see #42). Otherwise, I briefly tested your change in Spires and can confirm it's working as expected.

srenauld commented 3 years ago

Merged; I'll have a quick play tomorrow during alt keys before releasing.

Thank you very much for the contribution!