Questie

Questie

116M Downloads

Astrolabe.lua

AMGarkin opened this issue ยท 4 comments

commented

astrolabe/Astrolabe.lua

lines 184-201:

    if (not WorldMapFrame:IsVisible() == nil) then
        return
    else
        return Astrolabe_LastC, Astrolabe_LastZ, Astrolabe_LastX, Astrolabe_LastY
    end
    if (WorldMapFrame:IsVisible() == nil) then
        SetMapToCurrentZone()
        x, y = GetPlayerMapPosition("player")
        if (x <= 0 and y <= 0) then
            SetMapZoom(GetCurrentMapContinent())
            x, y = GetPlayerMapPosition("player")
            if (x <= 0 and y <= 0) then
                return
            end
        end
    else
        return Astrolabe_LastC, Astrolabe_LastZ, Astrolabe_LastX, Astrolabe_LastY
    end

These 2 conditions one after another doesn't make sense. Function will always end in the first one.
The first condition returns nil when coords are less then 0 when world map is open and last coordinates when map is closed. The second one tries to update player coordinates when map is closed and returns last coordinates when map is open. I believe that there should be just the second condition and the first should be deleted or commented out.
And maybe use if (not WorldMapFrame:IsVisible()) then instead of if (WorldMapFrame:IsVisible() == nil) then because return value from IsVisible() should be boolean.

lines 275, 276:

    Astrolabe.ForceNextUpdate = true;
    Astrolabe:UpdateMinimapIconPositions();

When you call UpdateMinimapIconPositions(), you should also reset update timer, otherwise you can have 2 updates in a row. My suggestion is:

    self.ForceNextUpdate = true
    self.UpdateTimer = self.MinimapUpdateTime
    self:UpdateMinimapIconPositions();

lines 528-531:

    for Z in ipairs(zones) do
        SetMapZoom(C, Z);
        zones[Z] = {mapFile = GetMapInfo(), mapName = N}
    end

There is missing N in for loop, so mapName will be always nil. Correct is: for Z, N in ipairs(zones) do

commented

Thanks for your input. In general we welcome code improvements of course, but since this is not related to an actual reproducible bug (if I understand you correctly) I hope you won't mind if this issue stays open for a bit. Unless someone else has the time to take care of it.

You are of course very welcome to create a PR.

commented

Yes, there are no bugs connected with Questie, however Astrolabe is used also in TomTom Vanilla and this version causes UI errors becuause of missing N (line 528). So if I want to use both addons together, I have to be sure that I have loaded version from TomTom (or my own version which has changes from both yours and theirs version).
I will try to create pull request.

commented

@AMGarkin I was reading this thread out of curiosity, and just leaving a sidenote:

If there is a problem in Astrolabe, and you need to make changes to Astrolabe.lua, then you must also ensure that your updated version will be loaded. Look at the top of Astrolabe.lua:

local LIBRARY_VERSION_MINOR = tonumber(string.match("$Revision: 90 $", "(%d+)") or 1)

This "Revision" needs to be bumped to a higher number to ensure your patched version is loaded instead of an older one.

Good luck! :-)

commented

It should be here: #524