Addon Etiquette

[ins]

Now I'm thinking I want code formatting like this forum has

Nice examples Vicster0. Ok if I copy some of that for http://www.esoui.com/downloads/info2...ddonTest.html? Was trying to make a sort of "Example for everything, in game, addon".

[Vicster0]

Originally Posted by ins Now I'm thinking I want code formatting like this forum has Nice examples Vicster0. Ok if I copy some of that for http://www.esoui.com/downloads/info2...ddonTest.html? Was trying to make a sort of "Example for everything, in game, addon".

By all means!

[Seerah]

There is no such thing as a "local global" variable.

Variables are in the global scope by default, unless they are declared as local by placing the word local before the declaration of the variable.

local myVariable = 5     --this is in the local scope

Local variables are available to the scope they are created in and anything below that scope.

local myVariable = 5
local function WhatIsMyVariable(var)
     local varToPrint = var
     print(varToPrint)
end
 
WhatIsMyVariable(myVariable)     --prints out 5 because myVariable and the local function were both defined in this scope (the main chunk of the Lua code)
WhatIsMyVariable(varToPrint)     --gives a nil error because varToPrint was defined at a lower scope (inside the function) it's not available at a higher scope (outside the function)

Variables should be declared local if and whenever possible. This is both to reduce the chance for collision with other leaked global variables with the same name by other authors, and for a boost in performance.

A variable should only be made global if access from outside of the file it is declared in is necessary. (Ie, referencing the function in an XML file, in-game debugging, support for other addons to have access...)

If a variable is in the global scope, it should have as unique a name as possible. This unique name should contain reference to your addon, since it's likely the user only has one addon with that name installed.

SavedVariables = {}     --bad - generically named global variable
MyCoolAddon_SavedVariables = {}     --good - uniquely named global variable

What most people miss is that frame names are GLOBAL REFERENCES to that frame. These need to be uniquely named objects as well.

local frame = CreateFrame("MyFrame", GuiRoot, CT_CONTROL)
--the local variable and the global reference both point to the same frame in memory
d(frame)     --prints out userdata:459efj9
d(MyFrame)     --prints out userdata:459efj9

If some other author also cleverly decides to give their control a global name of MyFrame, whichever one is created second will overwrite the first and the two addons will break each other. (The same is true for any other global variable.)

[Vicster0]

Originally Posted by Seerah There is no such thing as a "local global" variable.

Bah, you are correct. This is just my bad terminology... :P
At the end of the day, it is just a local variable BUT it can be local to the scope of your entire file, for instance.

Originally Posted by Seerah What most people miss is that frame names are GLOBAL REFERENCES to that frame. These need to be uniquely named objects as well. local frame = CreateFrame("MyFrame", GuiRoot, CT_CONTROL) --the local variable and the global reference both point to the same frame in memory d(frame)     --prints out userdata:459efj9 d(MyFrame)     --prints out userdata:459efj9 If some other author also cleverly decides to give their control a global name of MyFrame, whichever one is created second will overwrite the first and the two addons will break each other. (The same is true for any other global variable.)

This!!! Great thing to point out! You have to treat your frame names with the same level of consideration that you do any other global variable because they are exposed globally!

Honestly, I live by the rule that "It's better to be safe than sorry." If you are passing some sort of name string to a function and you are not entirely sure if it is global or local (and can't/won't do the leg work to figure it out) just use the MyAddonname_ sort of standard, as Seerah reiterated. Then you just don't have to be concerned. :P

[Joviex]

I'll be frank. Use namespaces.

You have a bunch of stuff you want "global" put it into a class i.e. namespace.


Does LUA allow you to reference items within your namespace(s) without full qualifiers? No.


But then you are not making namespaces like Viacom.Nickelodeon.West.Post


Just be smart. You use LibAddonMenu right? Most people probablly declare it as LAM


LAM: is your namespace/entry point.


Do the same thing to wrap "globals", dont go making a


defaults = {}


in the main module space when everyone and their uncle will do the same and cause a conflict.


MyModule.defaults = {}


I prefer dot notation, though I can see the argument for simply prepends of some prefix like MM_


Either case, yes, you SHOULD mangle/prefix/tag your stuff different from everyone elses.

[Vicster0]

Originally Posted by Joviex I'll be frank. Use namespaces. You have a bunch of stuff you want "global" put it into a class i.e. namespace. Does LUA allow you to reference items within your namespace(s) without full qualifiers? No. But then you are not making namespaces like Viacom.Nickelodeon.West.Post Just be smart. You use LibAddonMenu right? Most people probablly declare it as LAM LAM: is your namespace/entry point. Do the same thing to wrap "globals", dont go making a defaults = {} in the main module space when everyone and their uncle will do the same and cause a conflict. MyModule.defaults = {} I prefer dot notation, though I can see the argument for simply prepends of some prefix like MM_ Either case, yes, you SHOULD mangle/prefix/tag your stuff different from everyone elses.

Yup - I agree with this. I wouldn't (personally) put too much pressure on ONLY doing it this way but I do agree with this style. I think as long as you use something to make it as unique as possible and do as Seerah said and always try to make things local whenever you can, you are at least on the right track.

Also, pro-tip, when using tools like Zgoo, namespaces can/will save you LOADS of time debugging code.

[thelegendaryof]

Originally Posted by Vicster0 Ha, I remember the thread you are talking about... Great discussion going on here! I may be off my rocker, but I seriously think the biggest concern ANYONE should have regarding the standards/conventions being used with addon development would be poorly named global variables. I couldn't care less how awkward and convoluted your globals name is as long as it is unique enough to not pose potential overlaps with the default UI and other addons. As for your locals, you could obfuscate them to a letter table for all I care, tbh...

*cought* (backs off and checks his globals for possible retarded names and duplicates)

[Vicster0]

Originally Posted by thelegendaryof *cought* (backs off and checks his globals for possible retarded names and duplicates)

You're not alone...

Trust me, I rushed through developing InventoryInsight and I have been trying to balance my time between adding features and CLEANING the code lately. I know I shouldn't have any global that will interfere with other addons but they are also not necessarily standardized... So yes, even I am going back and trying to heed my own words.

[Stormknight]

Originally Posted by Vicster0 Also, pro-tip, when using tools like Zgoo, namespaces can/will save you LOADS of time debugging code.

Agreed, this saves me loads of time and should be enough incentive for any author to want to work in this manner.

Another one to add in here is slash commands.

I ran into this recently when I (as others have mentioned) was mashing code together quickly to get my first addon working. Awesome Info. As part of this, I created a slash command of /ai

Seemed reasonable at the time, then later other authors created addons with the same two letter slash command and suddenly there was an issue. AutoInvite is one.

I changed my slash command to something longer as I was only using it during debugging anyway.


So .... worth adding a reference page on the Wiki for slash commands and another for global namespace?

I don't know whether enough authors would check it to make it worthwhile is the problem.

[ins]

Originally Posted by Vicster0 You're not alone... Trust me, I rushed through developing InventoryInsight and I have been trying to balance my time between adding features and CLEANING the code lately. I know I shouldn't have any global that will interfere with other addons but they are also not necessarily standardized... So yes, even I am going back and trying to heed my own words.

I admitted defeat and redid the whole addon (the messiest one), as it was easier to write it from the ground up than fix a mess.

Only 4 more to go.

Edit:
Hot off the trail of the global/local talk.

Just had my first bug report as a bank addon and mine both had the same "function commandHandler "
Which apparently led to some issues when the user typed /mb to see commands in the other addon, and got for one of my first ****ty ones.

Did a quick fix to local everything, and moving functions around so they are reffering to each other in the right sequence (of code) and now I think that problem is out of the way.

Really really need to do a rewrite though.

[Vicster0]

Originally Posted by Stormknight Agreed, this saves me loads of time and should be enough incentive for any author to want to work in this manner. Another one to add in here is slash commands. I ran into this recently when I (as others have mentioned) was mashing code together quickly to get my first addon working. Awesome Info. As part of this, I created a slash command of /ai Seemed reasonable at the time, then later other authors created addons with the same two letter slash command and suddenly there was an issue. AutoInvite is one. I changed my slash command to something longer as I was only using it during debugging anyway. So .... worth adding a reference page on the Wiki for slash commands and another for global namespace? I don't know whether enough authors would check it to make it worthwhile is the problem.

This is exactly how it happens! I am currently using two slash commands for both of my addons but you are entirely right, the shorter ones probably just need to go away... Two letters is far too common and can/will cause future problems with other addons, INCLUDING your own!! LOL

I'm glad you brought this up though, I will make a note for that to be one thing to fix in this major version of InventoryInsight I'm working on right now.

Originally Posted by ins I admitted defeat and redid the whole addon (the messiest one), as it was easier to write it from the ground up than fix a mess. Only 4 more to go. Edit: Hot off the trail of the global/local talk. Just had my first bug report as a bank addon and mine both had the same "function commandHandler " Which apparently led to some issues when the user typed /mb to see commands in the other addon, and got for one of my first ****ty ones. Did a quick fix to local everything, and moving functions around so they are reffering to each other in the right sequence (of code) and now I think that problem is out of the way. Really really need to do a rewrite though.

Dude, that's funny. I want everyone to consider what was said here for a second... ESO has been live for a very short time and technically there are only a handful of addons for this game's UI at this time (compared to say WoW). With that being said, there are ALREADY overlap issues occurring. Even with addons from the same author!!

Better the mistakes be caught now and learned than to avalanche into the future. Thanks for sharing that with us Ins!

[Halja]

The interaction can be total strange too. Garkin and I had a brief add-on conflict. It had to do with the lua convention of dumping unused return values to an underscore character. Inside on of Skyshards locally decared functions was.

_,_,_,zone,subzone = string.find(textureName, "(maps/)(%w+)/(%w+_%w+)")

In ESOTheater, I had for whatever unknown reason had '_' in the relative parent position when setting an anchor.

tcontrol:SetAnchor(TOP, _, _, x, y)

If Skyshards loaded before mine, boom. The game tired to use the value from Skyshards on the set anchor. Easy to fix but very unexpected.

Cheers,
halja

[Vicster0]

Originally Posted by Halja The interaction can be total strange too. Garkin and I had a brief add-on conflict. It had to do with the lua convention of dumping unused return values to an underscore character. Inside on of Skyshards locally decared functions was. _,_,_,zone,subzone = string.find(textureName, "(maps/)(%w+)/(%w+_%w+)") In ESOTheater, I had for whatever unknown reason had '_' in the relative parent position when setting an anchor. tcontrol:SetAnchor(TOP, _, _, x, y) If Skyshards loaded before mine, boom. The game tired to use the value from Skyshards on the set anchor. Easy to fix but very unexpected. Cheers, halja

Pro-tip:

Only use '_,' during variable assignments. If your intention is to pass nothing, then pass nothing aka nil.

For example:

local someValue = 10
local _, val1, val2 = GiveMeThreeValues( nil, someValue )
 
function GiveMeThreeValues( arg1, arg2 )
    if( agr1 and arg2 ) then
        --arg1 ~= nil
        --arg2 ~= nil
        return arg1, arg2, (arg1 + arg2)
    end
    if( agr1 ) then
        --arg1 ~= nil
        return arg1, arg1, (arg1 + arg1)
    end
    if( agr2 ) then
        --arg2 ~= nil
        return arg2, arg2, (arg2 + arg2)
    end
    --just some nonesense for example...
end

I believe '_' is still a variable as far as LUA is concerned...
At least, this is how I see it...

[ins]

Originally Posted by Vicster0 Y Dude, that's funny. I want everyone to consider what was said here for a second... ESO has been live for a very short time and technically there are only a handful of addons for this game's UI at this time (compared to say WoW). With that being said, there are ALREADY overlap issues occurring. Even with addons from the same author!! Better the mistakes be caught now and learned than to avalanche into the future. Thanks for sharing that with us Ins!

Nono, it was my addon and BadVolts. The reason being I think was that we both used the same code example when we made our commandHandler - Edit: The wiki has a few examples using global functions instead of local as well.

Edit: Oh yea, Shardwatch apparently had the same global. Funny I've had both installed for ages and never ran into a problem with either, despite both functions named commandHandler.. So it is too now Local.

[Vicster0]

Originally Posted by ins Nono, it was my addon and BadVolts. The reason being I think was that we both used the same code example when we made our commandHandler - Edit: The wiki has a few examples using global functions instead of local as well. Edit: Oh yea, Shardwatch apparently had the same global. Funny I've had both installed for ages and never ran into a problem with either, despite both functions named commandHandler.. So it is too now Local.

Oh, I knew what you meant!

I'm only saying that, if that can happen because you both use the same template to create the function, what's to say you don't create the same function again, globally, and somewhat different in a separate addon. It has happened before. lol

[Seerah]

Originally Posted by Vicster0 I believe '_' is still a variable as far as LUA is concerned... At least, this is how I see it...

Yes. _ is just as valid a variable name as any other. It should be treated with just as much care (declaring it local) as any other variable you create/use. It's just convention that people use it as a "throw-away" variable when doing assignments from function returns. You can declare _ local at the top of your file, in the main chunk, and then use it 20 times if you want. No need to declare it local every time.

Originally Posted by Vicster0 If your intention is to pass nothing, then pass nothing aka nil.

Also, yes. It just worked in your testing because _ wasn't assigned to any value and equaled nil. It would work the same way if you used VariableIWantToStayNil instead of _. Well, until someone else leaked the same variable as a global...


/edit: oh yeah - and you can have more than one slash command for your addon. Just have them point to the same function. Leave the short one in as a shortcut if a user doesn't have another addon installed with the same command.

[Sasky]

Topic: Libraries
I agree with using LibStub or some way to insure an addon isn't included twice.

If a lib is optional, by all means leave the lib as a separate addon and just make sure it can handle missing the lib.

The other case I can see is when the lib changes frequently with improvements. Putting the lib as a separate addon can see faster updates of the addon -- but the addon should still contain some version of the lib.

If Minion is used by 99%+ of users and can handle dependencies, then we could go to an RPM/DEB/linux model with dependencies as separate packages. Until then, include some version if its required. Otherwise we could make a rather annoying chain the user has to follow (Addon requires some new menu lib builds on LibAddonMenu and needs that too, which needs you to get LibStub...)

Topic: Settings Names

Agree that some unified addons menu would be preferable since the list gets unwieldy to manage. Settings that you might want to change frequently should not make you go into the settings menu but have a slash command option.

Topic: Formatting

As long as it's self-consistent, it doesn't matter. When you start mixing tabs and spaces for your indenting, that's what gets annoying as tabs don't have a defined width (4 and 8 both common) so can throw the lines with spaces out of sync with the lines with tabs. If you have a collaborative addon with multiple authors working, then discuss indentation and naming conventions amongst yourselves.

Topic: Global Variables

Single table like in Vicster's post for the Addon provides the least exposure. At a minimum, it should be the addon name. If two addons have the same name, then that opens up tons of possible conflicts (ready function executed twice, confusion amongst users, files dumped in same folder, etc)

[Vicster0]

Originally Posted by Seerah /edit: oh yeah - and you can have more than one slash command for your addon. Just have them point to the same function. Leave the short one in as a shortcut if a user doesn't have another addon installed with the same command.

Oh yeah, you definitely can. However, I think I will be removing mine... I guess the thing here is, the entropy when you are dealing with 2-3 characters isn't much and EVENTUALLY someone is going to use the same shorthand slash command. (As in Stormknight's case for instance.) If your addon's default slash command is pretty long then maybe having a alternate with 6 or so characters would be nice and reasonable less dangerous due to the increased entropy. However, I think in some cases, likely frequent, shortening to 6 characters is really not that much better than the default. :P

Plus, I'm just saying, slash commands are kind old school... Maybe for backend dev/qa but for prod? I dunno.. lol LibAddonMenu is pretty freakin' simple...

And that's coming from someone who spends the better portion of his day in a CLI...

[Vuelhering]

Mostly just answering the OP. I'm late to this discussion I guess.


Re dependencies: Minion doesn't check dependencies. It probably should, but that does add a level of complexity. Individual addons can have dependencies.

Since you mentioned not including optional dependencies, many addons are dependent on LibAddonMenu and embed it in the download (which you suggested addons should not do). If you want a consistent look & feel, unlike Apple with Inside Mac, ZOS has not released a set of books (or even published their interfaces) or a set of libraries to help with look and feel. Even so, look & feel is ultimately up to the programming author. If ZOS wants a consistent look and feel, they can make a good set of libraries for it and a very clear, documented API that's not "discovered by the community and entered into a wiki" and doesn't require going to hacking and reverse-engineering sites to pull their Lua code out for examination just to write a program to support their product.

Re saved variables: Generally, no addons should intentionally touch another addon's saved variables. Any updates of the original can break everything. If an addon is designed solely to massage another addon's saved variables, absent some seriously transformative results, it's probably a derivative work of the original addon author. This includes things like bugfixes.

Re formatting: An addon is for end users, not for addon programmers. If you format like ****, you are the only one who suffers as the author. If it's buggy and can't be fixed easily by others, you also suffer as the author because nobody will fix it for you. If we trust the addon API, then we should never need worry about what is in the addon code.

I'm a big fan of releasing code and allowing others to maintain. But that doesn't mean I'll be held to someone else's standard of coding.

Frankly, I think all addons should be compiled or obfuscated by default, which will prevent issues that arose in WOW where people were stealing and distributing the addons then adding beg screens for money that didn't go to the original author, and it would also help prevent issues with copyright bickering when someone "borrows" 10 lines of code or something. I will also note, the day ZOS goes the way of blizzard and forbids obfuscated code is the day I will stop distributing addons (just like I did with blizzard with the Ni Karma System, a highly-acclaimed loot distribution system way back when). Bliz basically made a claim that "Your lua code runs on our interpreter which we got from Lua.org, therefore it's an extension of our game, therefore we can make a land-grab and try to steal your copyright and tell you what you can program in our stolen language, Lua." Tons of addon authors quit the game and stopped distributing addons. (Literally, tons. Including me, and I'm skinny. :-)

Re globals: Global variables should basically not be used, because they can have serious unintended side-effects by stepping on other things. I consider globals to be a memory leak. If they are necessary, they should be hidden within a single table clearly tied to your addon (and probably sharing the same name). If I wrote the Vuel Superwarlock addon, the only variable I should use is Superwarlock, where I can embed Superwarlock.config, Superwarlock.counter, etc.

[LilBudyWizer]

Overall a good idea. It's easy to say I'm just developing an addon all by my lonesome so what's it matter? I think that's the wrong way to view addons. It's crowd sourcing. If you're the only one that ever uses your addon then do whatever you please, but if you're going to release it then follow a few conventions.

Topic: Libraries

I have mixed feelings on libraries. Kudos for Seerah getting LibStub out early for a simple, but essential, function. LibStub doesn't require you do either, just one. The library can be a separate addon or included with an addon using it. So it's more convention. I think there's two cases where it's a good idea to be a separate library. One is media libraries or databases where the size can be substantial. The other is where the library provides an interface for the user. ZAM Stats is an example of that. It's not a library, but should be. It should be using LibStub and not ZAM_Stats. It really is a library with an example of using it.

So I don't think there's an always this way or that way answer, but, rather, it depends. I think there should certainly be guidelines to help developers understand when they should choose which.

Topic: Setting Names

By settings names I assume you mean what displays on the system/setting menu using LibAddonMenu. I certainly have encountered wtf is that? One problem though is what the user sees in the ADD-ONS menu is the Title from the manifest. So I think you need a standard for that, like "name - version{ - author}{ - description}. Name is the directory name for the addon. Version the version number as seen on ESOUI to make it easy to check when you need to update an addon. Author incase you have a bunch of addons from the same author to facilitate getting a list to check versions against. Description for when name is cryptic. The settings should then be description if present, otherwise name. It should be clear to the user what the settings are named from the addon list. You have a Description in the manifest so any additional information you want to add can be put there.

Separately LibAddonMenu needs to expand. Personally, all that should be under settings is "ADD-ONS". Clicking that then gives you a list of addons. That, preferably, a real list rather than a menu, i.e. like a file manager. You can filter, sort on columns, and such. Selecting an addon then pops menus like now, or alternatively a custom dialog. A custom dialog just handles some configurations better. I include that here because there needs to be some standard for that list. A way to pull the needed information from an addon. Not that I expect Seerah to do that anytime soon, but, presumably that addon is going to grow. Since lots of addons are already using it the ability pull the data as it grows rather than addons having to push it makes that easier. So I think there should be a convention for including data in a standard location for use by an addon manager. LibAddonMenu isn't the only possibility there. An example of an alternative one is an addon that tracks down who is polluting the global space by enabling and disabling addons and reloading the ui.

Topic: Saved Variables

I'm really baffled as to how Seerah avoids saved variables since, as far as I know, it's the only way to write a file. I assume Seerah actually means per character saved variables. My view is you use the simplest means possible to accomplish the task. When there is no actual sharing between characters then use per character rather than needlessly complicate it by adding subtables for users. All that accomplishes is changing the structure of the saved variables file. You can load those files into an LUA session external to the client. When there's no actual communication between characters make it clear by using the means provided.

I haven't really played with those function so I'm not sure what the parameters do. I understand it enough to use saved variables, but I'm not sure what happens with multiple calls and as a result of changing parameters when there's an existing saved variables to load. The details of that do matter. Some convention on session names, versions, defaults and profiles is needed. A convention for the name as well. Personally, addon_DB, addon_SV or addon_SavedVars where addon is the directory name for the addon. You can't really just have one global since saved variables is another. While you can pick up the variable name from the manifest it makes utilities a bit easier if you just use one.

Also within the saved variables a convention would be nice. Like if you need to share data between characters have "Characters" and "Account" keys. Account has stuff that applies to all characters while Characters has subkeys by the character name. Within those conventions such as options go in Options, history goes in History, data goes in Data, data to be imported in Import and data to be exported in Export. Export is for doing things like uploading your stats to a guild website. Import is from bring in databases such as node locations for addons such as Harvest Map. Some, like Options, should have some standards if certain options are supported, like window size and position.

Topic: Formating

Coding standards certainly. That, to me, would include naming, i.e. local variable/functions, global variables/functions, modules, keybinds, ect. Commenting as well. It should be easy for another programmer to pick up your addon. Some saves errors. Like I ran into naming a function the same as an API function that function called resulting in infinite recursion. I should have stuck them in a local table so there is no chance of such conflicts. As mentioned conventions also help with utilities. The programmer you screw may be yourself in depriving yourself of use of a handy tool.

I don't agree with stripping white space and such. Programmer automate thyself. If you want the white space stripped out then strip the white space out as you parse it. It's senseless to make the source difficult to read so a parser has an easier time at it. That's pennywise and dollar foolish. Spend hours trying to decipher illegible code to save yourself five minutes modifying the parsing process. The LUA manual lists the BNF definition of the language so you can cut and paste it into a parser. So, no, make it easy to read, not easy to parse. Conventions are just used for meta data, what's not in the code, i.e. a comment explaining what the purpose of the function is. Naming conventions are memory aids to the programmer.

Topic: Globals

There should be only one. Ok, maybe a few. It's a pointless debate across many languages. It's pointless because one variable can contain all your other globals. i.e. glob.this. glob.that and glob.theother. That said you actually need more for an addon. Your saved variables have to be global, the function you call creates them as globals. Otherwise they don't need to be. Your top level windows are global. Your key binding entry point has to be global. You need a global handle for the addon as well, logically the one global. One can debate whether than should be a table or class. Class rather complicates it for those that don't use classes. With those that do exposing a table is blasphemy. That one though should be the directory name for the addon.

I see far too much garbage polluting the global space. any()? Wtf is any? Any what? All I know is it dumps cryptic messages to the console and goes on and on and on. This is a particular problem at this stage. Documentation is incomplete and I have to browse the global table to find what I'm looking for. What I would really like to see is just one global for all addons. Every other addon then has a subtable within that. That would keep the global space as clean as possible.