Thread Tools Display Modes
04-23-14, 11:44 PM   #1
Digital_Utopia
A Flamescale Wyrmkin
 
Digital_Utopia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2012
Posts: 110
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
__________________
  Reply With Quote
04-24-14, 02:30 AM   #2
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,060
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).
  Reply With Quote
04-24-14, 11:36 AM   #3
Digital_Utopia
A Flamescale Wyrmkin
 
Digital_Utopia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2012
Posts: 110
Originally Posted by semlar View Post
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
__________________
  Reply With Quote
04-24-14, 03:52 PM   #4
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,060
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.
  Reply With Quote
04-24-14, 05:38 PM   #5
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
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.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)

Last edited by SDPhantom : 04-24-14 at 05:41 PM.
  Reply With Quote
04-24-14, 06:54 PM   #6
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,060
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?
  Reply With Quote
04-24-14, 08:57 PM   #7
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by semlar View Post
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.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote
04-24-14, 10:42 PM   #8
Digital_Utopia
A Flamescale Wyrmkin
 
Digital_Utopia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2012
Posts: 110
Originally Posted by semlar View Post
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.
__________________
  Reply With Quote
04-24-14, 11:16 PM   #9
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,060
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.

Originally Posted by Digital_Utopia View Post
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')

Last edited by semlar : 04-24-14 at 11:45 PM.
  Reply With Quote
04-24-14, 11:54 PM   #10
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by Digital_Utopia View Post
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.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote
04-25-14, 12:46 AM   #11
Digital_Utopia
A Flamescale Wyrmkin
 
Digital_Utopia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2012
Posts: 110
Originally Posted by semlar View Post
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.
__________________
  Reply With Quote
04-25-14, 01:10 AM   #12
Digital_Utopia
A Flamescale Wyrmkin
 
Digital_Utopia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2012
Posts: 110
Originally Posted by Phanx View Post
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.
__________________
  Reply With Quote

WoWInterface » Developer Discussions » General Authoring Discussion » Dear Addon I shall not name:

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off