View Single Post
04-03-14, 02:26 AM   #9
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Finally got a bit of time to look into this. I've only looked at a few of the files, but here's some feedback:

Originally Posted by cokedrivers View Post
How do I enable/disable a module?
module:SetEnabled(true|false)

Originally Posted by cokedrivers View Post
Am I using the Modules from Ace correctly?
Basically yes, but not if you intend for it to be possible to disable a module without reloading the UI. For example, in your Chat module, you overwrite a ton of globals, and set up several secure function hooks. If you try to disable the module as-is, nothing will happen -- all the globals will still be overwritten by the module's custom versions, and all the hooks will still be running. If this is how you want it to work, you're fine, but if you want users to be able to disable modules without reloading the UI, you need to do several things:

(1) Keep a copy of each global's original value before you overwrite it, eg. in a table:

Code:
local globals = {}

-- Save the old value if it wasn't already saved:
if not globals.CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA then
	globals.CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA = CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA
end
-- before you set the new value:
CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA = 1
To make this easier since you're doing this to a lot of globals, you could use a helper function:

Code:
-- Put this near the top of your file, outside of any functions:
local globals = {}
local function setglobal(k, v)
	globals[k] = globals[k] or _G[k] -- here is an actual valid use of _G
	_G[k] = v
end

-- Then do this in your OnEnable function:
setglobal("CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA", 1)
setglobal("CHAT_FRAME_TAB_SELECTED_NOMOUSE_ALPHA", 0)
Then in your module's OnDisable function (which you'd need to add, as you don't have one currently) go through the table and restore all the globals to their original values:

Code:
for k, v in pairs(globals) do
	_G[k] = v
end
The note about a valid use of _G in the comment of the code above refers to you doing things like this:

Code:
_G.CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA = 1
This is totally pointless, and just makes your code cluttered. These globals are being set once when your addon is enabled, so there's no need to micro-optimize here. Just access them directly:

Code:
CHAT_FRAME_TAB_SELECTED_MOUSEOVER_ALPHA = 1
Non-secure function/method hooks are also easy to back up and restore:

Code:
-- At the top of the file:
local methods = {}

-- In your ModChat function, store the original "AddMessage" method for each frame:
methods[chat] = methods[chat] or {}
methods[chat].AddMessage = chat.AddMessage
-- ... before you overwrite it:
chat.AddMessage = FCF_AddMessage
Then in your OnDisable function, go through and restore everything:

Code:
for frame, t in pairs(methods) do
	for method, func in pairs(t) do
		frame[method] = func
	end
end
Secure hooks are slightly more complicated, as there's no way to actually remove them. Create a variable to store your module's enable state, and define the hook function, outside of your other functions:

Code:
local DISABLED = true

function MyModule.ChatFrameMenu_UpdateAnchorPoint()
	if DISABLED then return end
	if FCF_GetButtonSide(DEFAULT_CHAT_FRAME) == 'right' then
		ChatMenu:ClearAllPoints()
		ChatMenu:SetPoint('BOTTOMRIGHT', ChatFrame1Tab, 'TOPLEFT')
	else
		ChatMenu:ClearAllPoints()
		ChatMenu:SetPoint('BOTTOMLEFT', ChatFrame1Tab, 'TOPRIGHT')
	end
end
In your OnEnable and OnDisable functions, set the value of the DISABLED variable to false or true, respectively.

Just set up the hook once in your OnEnable function, and remove the reference to the function as a member of your module as a means of keeping track of whether it's been set already:

Code:
	if self.ChatFrameMenu_UpdateAnchorPoint then
		hooksecurefunc("ChatFrameMenu_UpdateAnchorPoint", self.ChatFrameMenu_UpdateAnchorPoint)
		self.ChatFrameMenu_UpdateAnchorPoint = nil
	end
Since functions are passed by reference, this doesn't actually delete the function, it just removes the ability to access it via self.ChatFrameMenu_UpdateAnchorPoint.

However, you should look into whether it's actually necessary to use a secure hook here; for example, it's not necessary -- and also much easier -- to use an insecure hook for FCF_OpenTemporaryWindow:

Code:
	local FCF_OTW = FCF_OpenTemporaryWindow
	function FCF_OpenTemporaryWindow = function(...)
		-- Call the original function with whatever arguments were passed in,
		-- and capture the returned frame reference:
		local window = FCF_OTW(...)
		-- Pass the frame to your function:
		SetChatStyle(window)
		-- And finally return the frame:
		return window
	end
Then you can remove all that looping business from your SetChatStyle function, and just check if the provided chat frame was already modified:

Code:
local function SetChatStyle(chat)
	if not chat or chat.hasModification then return end
	
	-- do the rest of the stuff here
end
Finally, rather than defining all of these local functions inside your OnEnable function, you'll need to move them outside of it, or make them methods of your module, eg:

Code:
function BasicUI_Chat:SetChatStyle(chat)
	-- do stuff
end
Then call those methods instead of the local functions. The advantage of using methods is that you can organize your code however you want, without worrying about whether the function is defined above the line where you call it.
__________________
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