WoWInterface

WoWInterface (https://www.wowinterface.com/forums/index.php)
-   General Authoring Discussion (https://www.wowinterface.com/forums/forumdisplay.php?f=20)
-   -   Dear Addon I shall not name: (https://www.wowinterface.com/forums/showthread.php?t=49231)

Digital_Utopia 04-23-14 11:44 PM

Dear Addon I shall not name:
 
Why do you make me do this?

Code:

local zo=_G.ZoomOut;
function ZoomOut()
        if(not WorldMapFrame:IsVisible())then
                SetMapToCurrentZone()
                local dl,x1,y1,x2,y2=GetCurrentMapDungeonLevel();
                map:updateInfo(GetCurrentMapAreaID(),dl)
                local mapName, textureWidth, textureHeight, isMicroDungeon, microDungeonName = GetMapInfo()
                indoors=isMicroDungeon;
                zoomChanged=true;
        end
        zo();
end


semlar 04-24-14 02:30 AM

I would strongly recommend against overwriting the ZoomOut function because that is going to taint the world map.

You're also changing the behavior of ZoomOut to the point where it won't function correctly.

If ZoomOut is called from a map other than the current one, you're setting it to the current map then calling ZoomOut, so it will only ever switch to the map above the current one, and you aren't returning anything (it's supposed to return whether it can zoom out or not).

Digital_Utopia 04-24-14 11:36 AM

Quote:

Originally Posted by semlar (Post 292311)
I would strongly recommend against overwriting the ZoomOut function because that is going to taint the world map.

You're also changing the behavior of ZoomOut to the point where it won't function correctly.

If ZoomOut is called from a map other than the current one, you're setting it to the current map then calling ZoomOut, so it will only ever switch to the map above the current one, and you aren't returning anything (it's supposed to return whether it can zoom out or not).

I honestly would prefer if it didn't seem that I had to do this, and if someone has a better way, I'm certainly all ears; but every time ZONE_CHANGED_NEW_AREA fires, this addon checks whether or not the player is in a MicroDungeon, and if so - calls ZoomOut. Which, in turn, fires off another ZCNA event.

This means, that for every ZCNA event, while inside a microdungeon, the first call to GetCurrentMapAreaID will deliver the correct mapID, while subsequent ones will all return the zoomed out mapID.

My first attempt was to use hooksecurefunc on ZoomOut, which would at least let me know when ZoomOut was called; but since it's a post-hook, the ZCNA event would already fire, and calling SetMapToCurrentZone would result in a tug of war of sorts with this other addon.

Basically, on the hook function, I'd call SetMapToCurrentZone, which would fire the ZCNA event, which the other addon would respond to, by calling ZoomOut, which would get hooked, and the entire process repeats indefinitely.

By pre-hooking/overriding, I can get the information I should be able to get, allow the other addons to do their own thing, and ignore the results of their actions.

But thanks for pointing out the issues with the above. Neither WoWpedia or wowprogramming.com mentioned a return value for ZoomOut; but that was easy enough to address. The second issue, regarding the altering of the map where the ZoomOut would be called on, was also an oversight. While I'd hope that other addons aren't going to strong-arm a map change, I understand that it's fairly common to traverse the map while it's hidden, to get one-time information - something I don't want to interfere with.

So, the result is as such:

Code:

local zo=_G.ZoomOut;
function ZoomOut()
        if(not WorldMapFrame:IsVisible())then
                local z =GetCurrentMapAreaID()
                SetMapToCurrentZone()
                local z2 = GetCurrentMapAreaID();
                local dl,x1,y1,x2,y2=GetCurrentMapDungeonLevel();
                map:updateInfo(GetCurrentMapAreaID(),dl)
                local mn, tw, th, imd, mdn = GetMapInfo()
                indoors=imd;
                zoomChanged=true;
                if(z~=z2)then
                        SetMapByID(z);
                end
        end
        return zo();
end


semlar 04-24-14 03:52 PM

You shouldn't need to SetMapToCurrentZone every time another addon tries to change the map.

Even if you need to request information from the current map OnUpdate, you would only need to set the map once, gather your information, then allow the other addon to zoom out and do whatever it's doing without breaking yours because you already did what you needed to.

Without knowing what the other addon is or what it's doing, or what your addon needs to do it's difficult to suggest what the best course of action would be.

SDPhantom 04-24-14 05:38 PM

If you really need to, you can briefly unregister an event for all frames then re-register when done.

Lua Code:
  1. local Frames={GetFramesRegisteredForEvent("ZONE_CHANGED_NEW_AREA")};
  2. for i,j in ipairs(Frames) do j:UnregisterEvent("ZONE_CHANGED_NEW_AREA"); end
  3.  
  4. --  Do whatever you need here
  5.  
  6. for i,j in ipairs(Frames) do j:RegisterEvent("ZONE_CHANGED_NEW_AREA"); end

I wouldn't suggest doing this very often as it creates a new table each time it's run.
For the sake of simplicity, I didn't include a table recycling function to use in such case.

semlar 04-24-14 06:54 PM

This can also be done without a table by using select or a recursive function.

As for efficiency, I can't say whether the overhead of the function calls would outweigh dealing with recycling a table. It's probably simpler to write, at any rate.
Lua Code:
  1. local function ProcessFrames(...)
  2.     local n = select('#', ...)
  3.     for i = 1, n do select(i, ...):UnregisterEvent('ZONE_CHANGED_NEW_AREA') end
  4.    
  5.     -- Do stuff
  6.    
  7.     for i = 1, n do select(i, ...):RegisterEvent('ZONE_CHANGED_NEW_AREA') end
  8. end
  9.  
  10. ProcessFrames(GetFramesRegisteredForEvent('ZONE_CHANGED_NEW_AREA'))
More importantly, I'm unsure if there's potential for a race condition by doing this.

What happens if the event fires while you're unregistering frames?

Phanx 04-24-14 08:57 PM

Quote:

Originally Posted by semlar (Post 292314)
You shouldn't need to SetMapToCurrentZone every time another addon tries to change the map.

Not only should not not need to do this, but you shouldn't do it at all, as doing so will likely break the other addons, which are almost certainly changing the map for a reason.

If there is a particular addon that's poorly coded and breaking things, the solution isn't to break more things -- the solution is to tell that addon's author about the problem and ask them to fix it, or simply stop using that addon.

Digital_Utopia 04-24-14 10:42 PM

Quote:

Originally Posted by semlar (Post 292314)
You shouldn't need to SetMapToCurrentZone every time another addon tries to change the map.

Even if you need to request information from the current map OnUpdate, you would only need to set the map once, gather your information, then allow the other addon to zoom out and do whatever it's doing without breaking yours because you already did what you needed to.

Without knowing what the other addon is or what it's doing, or what your addon needs to do it's difficult to suggest what the best course of action would be.

Ideally, yes. That's been my goal all along. Unfortunately, the ZCNA event fires on any change to the map whatsoever - actual zone changes, which was the (presumed) intent, player map navigation, and API functions that set the map (SetMapToCurrentZone, SetMapByID, and ZoomOut). So the problem here, essentially, is determining which of these results are accurate, and which are done by artificial means.

So while getting the proper zone at any one point is easy enough, continuing to get the proper zone is not. And I'm really not talking about the results of player interaction with the world map - that's a given, and an understandable concession.

When it comes to other addons, and actions taken while the WorldMap is closed, that's a bit different. Now, there's absolutely no big deal with changing the map, while it's closed, to get information from a different zone, so long as you set it back when you're done. The entire process would take less than a single frame, and other addons shouldn't be affected at all.

But in this case, the addon is actually strong-arming its map change. Meaning, as long as the player is in a micro dungeon, it will make sure the map stays zoomed out to the main area. And every time it does that, it will cause the ZCNA event to fire.

So if you step into Shrine of Two Moons, you'll get an initial ZCNA fire, with the areaID showing as 903. But then you'll immediately get another ZCNA fire, with the areaID showing as 811 (Vale). Now, storing the first one is easy enough, but what happens when the player steps out? The areaID will still be 811, and there's no way (I know of) to tell what caused the ZCNA event to fire.

My main goal is to simply create a addon-scoped, psuedo event & function, so the other parts of my addon can easily get map information on demand, or on change. Without having to register and filter Blizzard events for each section. It worked great by itself, and with every other addon I've got installed - except this one. /shrug

As far as the other addon goes - I'd prefer to keep the name to myself. My goal for this thread isn't to call out the author or the addon, but to express some mild irritation of having to compensate for it, and what it seems I had to do. I understand why they did this - so microdungeons could be treated like other inside areas, and not in their own zone - so outside map related items could still be shown while the player is inside. I get it, I really do - but if they're going to do that, it would be nice if they at least provided an addon-scoped table or function to provide the information they're essentially blocking by doing this. If not just go with Astrolabe, which can do this without such...methods.

semlar 04-24-14 11:16 PM

Why do you need to force the map back to the current zone every time it gets changed?

If you need information about the map the player is currently on, you only need to call SetMapToCurrentZone in response to a few zone events, store whatever information you need in a table, and after that it doesn't matter what anything does to the map until the next zone event.

Quote:

Originally Posted by Digital_Utopia (Post 292320)
Unfortunately, the ZCNA event fires on any change to the map whatsoever - actual zone changes, which was the (presumed) intent, player map navigation, and API functions that set the map (SetMapToCurrentZone, SetMapByID, and ZoomOut).

ZONE_CHANGED_NEW_AREA doesn't fire when you switch maps, that only triggers WORLD_MAP_UPDATE.

Here's a small example to demonstrate when each event actually fires..
Lua Code:
  1. local f = CreateFrame('frame')
  2. f:SetScript('OnEvent', function(self, event, ...)
  3.     print(event, GetRealZoneText(), GetSubZoneText())
  4. end)
  5. f:RegisterEvent('ZONE_CHANGED_NEW_AREA')
  6. f:RegisterEvent('ZONE_CHANGED_INDOORS')
  7. f:RegisterEvent('ZONE_CHANGED')
  8. f:RegisterEvent('NEW_WMO_CHUNK')

Phanx 04-24-14 11:54 PM

Quote:

Originally Posted by Digital_Utopia (Post 292320)
As far as the other addon goes - I'd prefer to keep the name to myself. My goal for this thread isn't to call out the author or the addon, ...

If their addon is doing what you say it's doing, that's a problem that should be fixed, and you should at least tell the addon's author. Most likely they have no idea that what they're doing is a problem, or they wouldn't be doing it in the first place. Just silently working around it doesn't help anybody.

Digital_Utopia 04-25-14 12:46 AM

Quote:

Originally Posted by semlar (Post 292321)
Why do you need to force the map back to the current zone every time it gets changed?

If you need information about the map the player is currently on, you only need to call SetMapToCurrentZone in response to a few zone events, store whatever information you need in a table, and after that it doesn't matter what anything does to the map until the next zone event.


ZONE_CHANGED_NEW_AREA doesn't fire when you switch maps, that only triggers WORLD_MAP_UPDATE.

Hey, guess what? I found the problem!

Basically, I'm an idiot.

It's amazing the kind of trouble a malformed if statement can cause. Because of that, a registered, but never handled WORLD_MAP_UPDATE leaked through into the handler for my ZCNA event. Oh well, I'll get this figured out yet. Sorry I had to drag you guys into my noob moment, but thanks anyway for the help.

Digital_Utopia 04-25-14 01:10 AM

Quote:

Originally Posted by Phanx (Post 292322)
If their addon is doing what you say it's doing, that's a problem that should be fixed, and you should at least tell the addon's author. Most likely they have no idea that what they're doing is a problem, or they wouldn't be doing it in the first place. Just silently working around it doesn't help anybody.

Should go without saying now, but the other reason why I didn't want to start throwing out names, is because I'm nowhere near proficient enough with lua or the API to be above some really stupid mistakes. While it is true that this addon will call zoomout whenever the ZCNA event fires, and the player is in a microdungeon, it was my own screwup that caused this action to well, really make any difference.


All times are GMT -6. The time now is 07:19 AM.

vBulletin © 2024, Jelsoft Enterprises Ltd
© 2004 - 2022 MMOUI