Thread Tools Display Modes
10-30-14, 04:24 PM   #1
gmarco
An Onyxian Warder
 
gmarco's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 362
Global vars ?

That's the lazy solution, but really you should just make sure you're not leaking any globals. Anywhere you define a variable, stick a "local" in front of it. If you need it accessible in a higher scope, define it in that higher scope with no value (just "local myVariable") and then give it its value later, but as a general rule, keep variables in the narrowest scope necessary. If it only needs to exist inside a certain function, make sure it's defined as local inside that function. If it only needs to exist inside a certain for loop, keep it inside the loop, etc.

You can use WowGlobalFinder to help you find all the leaks in your code.
Thanks for the reply Phanx,
I opened this new post because I like to discuss about this a little bit more if possible....

Is there a reason to use a variable as NON LOCAL ?

For example ... another addon of mine: Aggromon uses some saved variables for the configuration option.
These are defined in the .toc file:

Lua Code:
  1. ## SavedVariables: AGGROMON_ACTIVE, AGGROMON_SOUND, AGGROMON_CHAT, AGGROMON_SHOW, AGGROMON_SNDFLE, AGGROMON_ENAPVP

and in the addon I have:

Lua Code:
  1. local ADDON = ...
  2.  
  3. -- defaults if not founds values
  4. AGGROMON_ACTIVE = AGGROMON_ACTIVE or true
  5. AGGROMON_SOUND = AGGROMON_SOUND or true
  6. AGGROMON_CHAT = AGGROMON_CHAT or false
  7. AGGROMON_SHOW = AGGROMON_SHOW or false
  8. AGGROMON_SNDFLE = AGGROMON_SNDFLE or 1
  9. AGGROMON_ENAPVP = AGGROMON_ENAPVP or false
  10.  
  11. -- etc etc ...

I used them as non local, but the names should be fine to not break any others things in the UI.

The question is :-)
Is it fine or I should define them as local even if I am exporting outside the addon ...

Thanks again to all of you.

P.s
Checking better I have also this one :-)

Lua Code:
  1. SLASH_AGGROMON1 = "/aggromon";
  2. SlashCmdList["AGGROMON"] = function()
  3.     InterfaceOptionsFrame_OpenToCategory(options)
  4. end
__________________
This is Unix-Land. In quiet nights, you can hear the Windows machines reboot.

Last edited by gmarco : 10-30-14 at 04:29 PM.
  Reply With Quote
10-30-14, 05:03 PM   #2
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,290
Originally Posted by gmarco View Post
Thanks for the reply Phanx,
I opened this new post because I like to discuss about this a little bit more if possible....

Is there a reason to use a variable as NON LOCAL ?

For example ... another addon of mine: Aggromon uses some saved variables for the configuration option.
These are defined in the .toc file:

Lua Code:
  1. ## SavedVariables: AGGROMON_ACTIVE, AGGROMON_SOUND, AGGROMON_CHAT, AGGROMON_SHOW, AGGROMON_SNDFLE, AGGROMON_ENAPVP

and in the addon I have:

Lua Code:
  1. local ADDON = ...
  2.  
  3. -- defaults if not founds values
  4. AGGROMON_ACTIVE = AGGROMON_ACTIVE or true
  5. AGGROMON_SOUND = AGGROMON_SOUND or true
  6. AGGROMON_CHAT = AGGROMON_CHAT or false
  7. AGGROMON_SHOW = AGGROMON_SHOW or false
  8. AGGROMON_SNDFLE = AGGROMON_SNDFLE or 1
  9. AGGROMON_ENAPVP = AGGROMON_ENAPVP or false
  10.  
  11. -- etc etc ...

I used them as non local, but the names should be fine to not break any others things in the UI.

The question is :-)
Is it fine or I should define them as local even if I am exporting outside the addon ...

Thanks again to all of you.

P.s
Checking better I have also this one :-)

Lua Code:
  1. SLASH_AGGROMON1 = "/aggromon";
  2. SlashCmdList["AGGROMON"] = function()
  3.     InterfaceOptionsFrame_OpenToCategory(options)
  4. end
Depends on your number of vars, and addon structure. If you have a lot of saved vars to work with, it's better to slap into the addon table, so you can reach them faster as self.randomsavedvar. You could also reduce that 6 globals into 1 if you store all the 6 variables in 1 global table, like AGGROMON[ACTIVE] etc.

Slash commands must be global, else the user won't be able to use them.
  Reply With Quote
10-30-14, 07:02 PM   #3
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by gmarco View Post
Is there a reason to use a variable as NON LOCAL ?
Object's tags (like CreateFrame("Frame", "FrameTag", ...); ), SavedVariablesm, slash and addon build-in table (like local t, n = ...; ADDON_NAME = n; )

Last edited by AlleyKat : 10-30-14 at 07:09 PM.
  Reply With Quote
10-31-14, 09:15 PM   #4
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Most variables you create and use in the course of writing an addon should remain local. There are very few situations where a global variable is necessary or desirable. You should get to know what those situations are, and avoid using globals in any other situations.

Saved variables are necessarily global. If your addon uses more than ~3 saved variables, as in the example you posted, it's probably better to do as Resike said and switch over to saving a single table with all those things inside it:

Code:
## SavedVariables: AggroMonDB
Code:
AggroMonDB = {
     active = true,
     sound = true,
     chat = true,
     show = true,
     soundFile = "ds\\sound\\creature\\murloc\\mmurlocaggroold.ogg",
     enablePVP = false,
}
The names you give frames and other UI objects are also global:

Code:
CreateFrame("Frame", "MyFrame", UIParent)
MyFrame:CreateFontString("MyFontString", "OVERLAY")
A good rule of thumb here is to give global names only to:

(a) secure objects like action buttons,
(b) objects that inherit from a Blizzard templates like ActionButtonTemplate that expect a name, and
(c) your addon's main frame object.

For everything else, just pass nil where the Create* API accepts a name, and use local references:

Code:
local frame = CreateFrame("Frame", nil, UIParent)
local text = MyFrame:CreateFontString(nil, "OVERLAY")
frame.text = text
^ In your code, you'd want to use the "frame" and "text" variables; the last line just makes it possible to access the font string easily from outside that file, eg. with a /run command in-game.

Slash commands are also necessarily global:

Code:
SLASH_MYADDON1 = "/myaddon"
So are binding and binding header names:

Code:
BINDING_HEADER_MYADDON = "MyAddon"
BINDING_NAME_MYADDON_TOGGLEFRAME = "Toggle frame"
However:

Originally Posted by AlleyKat View Post
... addon build-in table (like local t, n = ...; ADDON_NAME = n; )
This is incorrect for two reasons:

1. The values passed to your addon files are not globals. They're not even assigned to variables at all unless you assign them to variables yourself.

2. Your example is backwards; the first value passed into each file is the addon's folder/TOC name, and the second is a table. There's also no reason whatsoever to assign the value to "n" and then assign it to "ADDON_NAME" immediately afterwards, and your example is putting that "ADDON_NAME" in the global namespace, which is a horrible idea.

Code:
local ADDON_NAME, privateTable = ...
ADDON_NAME is a string, eg. "MyAddon" or "oUF_Phanx_Config", that matches the name of your addon's folder and TOC file.

privateTable is a table. It starts out empty, but you can add whatever you want to it, and those contents are then available in all of your addon's files, since all references point to the same table object.
__________________
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.

Last edited by Phanx : 10-31-14 at 09:19 PM.
  Reply With Quote
10-31-14, 09:43 PM   #5
myrroddin
A Pyroguard Emberseer
 
myrroddin's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2008
Posts: 1,240
One other reason to use global scope variables is if your AddOn creates APIs that a separate AddOn might use. For example, HandyNotes has several globals, which is how any plugins gain access.

TomTom has some global APIs that other AddOns can use to add waypoints to TomTom.

However, in most cases it is best to create AddOns as being as much self-enclosed as possible.
  Reply With Quote
11-01-14, 01:11 AM   #6
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
You're confusing methods with global variables. There's only one global here:

Code:
function TomTom:SetClosestWaypoint(...)
end

function TomTom:AddMFWaypoint(...)
end

function TomTom:AddZWaypoint(...)
end
Those method names -- AddMFWaypoint, AddZWaypoint, SetClosestWaypoint -- are not global variables. They're not local variables, either. They're table keys. Another way to visualize the situation would be like this:

Code:
TomTom = {
     SetClosestWaypoint = function(TomTom, ...)
     end,
     SetMFWaypoint = function(TomTom, ...)
     end,
     SetZWaypoint = function(TomTom, ...)
     end,
}
This falls under the heading of "your addon's main object" -- in TomTom's case, its main object is a table, and is accessible via the global variable "TomTom". Everything else in your addon -- methods, other frames, font strings, textures, etc. -- should either be strictly local (if you don't need to access them outside of the file where they're defined) or added to your main frame/table (the "frame.text" in my previous post) so it can be accessed through the main object.
__________________
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.

Last edited by Phanx : 11-01-14 at 01:14 AM.
  Reply With Quote
11-01-14, 11:27 PM   #7
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by Phanx View Post
This is incorrect for two reasons
No, this way is correct to expose your addon to other addons
local t, n = ...;
MY_ADDON = n;
or

Code:
local t, n = ...;
_G[t] = n;
or my way
Code:
local title, M = ...;
local frame = CreateFrame("Frame", title);
M[0] = frame[0];
	
setmetatable(M, { __index = getmetatable(frame).__index });
setmetatable(frame, { __index = M, __newindex = M });

Last edited by AlleyKat : 11-01-14 at 11:31 PM.
  Reply With Quote
11-02-14, 12:49 AM   #8
Voyager
A Fallenroot Satyr
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 22
Originally Posted by AlleyKat View Post
or my way
Code:
local title, M = ...;
local frame = CreateFrame("Frame", title);
M[0] = frame[0];
	
setmetatable(M, { __index = getmetatable(frame).__index });
setmetatable(frame, { __index = M, __newindex = M });
What's the point of the red parts?
  Reply With Quote
11-02-14, 01:33 AM   #9
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by Voyager View Post
What's the point of the red parts?
Code:
local title, M = ...;

M:SetScript("OnEvent", function(self, ...)
-- ...
-- where self ~= M, but self[key] == M[key]
end);
my build-in table now acting like frame
and frame with addon's name tag now acting like build-in table, that acting like frame
and now my build-in table has frame methods

Last edited by AlleyKat : 11-02-14 at 01:46 AM.
  Reply With Quote
11-02-14, 01:41 AM   #10
Voyager
A Fallenroot Satyr
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 22
Originally Posted by AlleyKat View Post
Code:
local title, M = ...;

M:RegisterEvent(event, function(self, ...)
-- ...
-- where self ~= M, but self[key] == M[key]
end);
my build-in table now acting like frame
and frame with addon's name tag now acting like build-in table, that acting like frame
Couldn't one just do the following?
Code:
local title, M = ...;
M = CreateFrame("Frame", title);

M:RegisterEvent(event, function(self, ...)
-- ...
-- where self == M and self[key] == M[key]
end);
  Reply With Quote
11-02-14, 01:45 AM   #11
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Ah, my RegisterEvent function not from frame
  Reply With Quote
11-02-14, 01:48 AM   #12
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by Voyager View Post
Couldn't one just do the following?
Code:
local title, M = ...;
M = CreateFrame("Frame", title);

M:RegisterEvent(event, function(self, ...)
-- ...
-- where self == M and self[key] == M[key]
end);
now key from frame ~= key from build-in table
  Reply With Quote
11-02-14, 08:28 PM   #13
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by AlleyKat View Post
or my way
Code:
local title, M = ...;
local frame = CreateFrame("Frame", title);
M[0] = frame[0];
	
setmetatable(M, { __index = getmetatable(frame).__index });
setmetatable(frame, { __index = M, __newindex = M });
This is incredibly overcomplicated and pointless... if you want your globally-accessible object to be a frame, just make it a frame, and don't use the private table. The private table is only there so that if you don't want a global, you don't need to make one just to share data between files in your 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
11-02-14, 10:43 PM   #14
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by Phanx View Post
This is incredibly overcomplicated and pointless... if you want your globally-accessible object to be a frame, just make it a frame, and don't use the private table. The private table is only there so that if you don't want a global, you don't need to make one just to share data between files in your addon.
I like to store my data in build-in table, also I like to use my main storage as frame-type-table (events and etc..), and my storage must be available outside. What is pointless? Make a lot of global variables, tables and frames or use one extended table (build-in)?

Last edited by AlleyKat : 11-02-14 at 10:46 PM.
  Reply With Quote
11-03-14, 01:17 AM   #15
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by AlleyKat View Post
I like to store my data in build-in table, also I like to use my main storage as frame-type-table (events and etc..), and my storage must be available outside. What is pointless? Make a lot of global variables, tables and frames or use one extended table (build-in)?
Your method still creates a frame object in addition to the private table. Using multiple layers of metatables and trickery to make the private table identical to the frame is pointless and inefficient. Just use the frame, and don't use the private table. That doesn't create any more global variables, tables, or frames than your current convoluted method.
__________________
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
11-03-14, 01:45 AM   #16
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
In my way I can access to my frame via
Code:
local title, M = ...;
in your way I must

Code:
local title = ...;
local M = _G[title];

Last edited by AlleyKat : 11-03-14 at 01:48 AM.
  Reply With Quote
11-03-14, 01:53 AM   #17
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Originally Posted by Phanx View Post
Using multiple layers of metatables and trickery to make the private table identical to the frame
Standard metatable functions
in frame case we redirect all get and set notfound operations to build-in
in build-in we adding frame api
and now we don't care what table to use, coz they are same

Last edited by AlleyKat : 11-03-14 at 02:14 AM.
  Reply With Quote
11-03-14, 02:26 AM   #18
AlleyKat
A Warpwood Thunder Caller
 
AlleyKat's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2010
Posts: 93
Btw my method is used only to extend build-in table, not to make it global, makes it global - side affect of frame, and I don't recommend use it only to make your table global
  Reply With Quote
11-03-14, 05:24 AM   #19
Lombra
A Molten Giant
 
Lombra's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 554
Originally Posted by AlleyKat View Post
In my way I can access to my frame via
Code:
local title, M = ...;
in your way I must

Code:
local title = ...;
local M = _G[title];
Just do this then:
Code:
local title, M = ...;
M.frame = CreateFrame("Frame")
__________________
Grab your sword and fight the Horde!
  Reply With Quote
11-03-14, 06:25 AM   #20
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Originally Posted by AlleyKat View Post
in your way I must
Code:
local title = ...;
local M = _G[title];
So... adding one line of code is worse, in your mind, than adding multiple extra lines of code and adding the overhead of multiple metatables in your original example?

Originally Posted by AlleyKat View Post
Standard metatable functions
in frame case we redirect all get and set notfound operations to build-in
in build-in we adding frame api
and now we don't care what table to use, coz they are same
This is just bad programming. If you want to be able to use two variables to refer to the "same" object, just create one object and point two variables to it. Creating two objects and using metatables to make them act like the same object is just ... I don't even know where you came up with such a convoluted idea. There's no benefit to doing this whatsoever. Then again, there's no benefit to having two variables in the same scope that refer to the "same" object, either, other than purposefully making your code more complicated and difficult to read.
__________________
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

WoWInterface » Developer Discussions » Lua/XML Help » Global vars ?

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