[Enhancement] Avoid global namespace pollution

[Road-block]

The saved variables names selected are too generic.

MiniMapDB KeyboardPosition MousePosition ShowEmptyBinds ShowInterfaceBinds KeyBindSettings KeyBindSettingsMouse MouseKeyEditLayouts KeyboardEditLayouts CurrentLayoutMouse CurrentLayoutKeyboard tutorialCompleted

These are all put in the global namespace after ADDON_LOADED fires for KeyUI. That means they can overwrite or get overwritten by other addons leading to really hard to debug issues.

I'm not mentioning the KeyUI_Settings variable because that one is actually properly named <addonname>_suffix is a good naming scheme because the addonname is always unique for a specific player's installation (you cannot have 2 folders with the same name in \AddOns).

The solution to this problem can be one of

• Prefix the existing variables with KeyUI_
• Tuck them inside the KeyUI_Settings variable (I would go with this) The second one will also make the variables migration quite easy.

So they would be KeyUI_Settings.MinimapDB KeyUI_Settings.KeyboardPosition .. etc.

[1onar]

“Thank you for your advice! This is my first coding project, so I’m still learning as I go. I really appreciate your feedback and am open to any suggestions you may have!

[Road-block]

It's a well made addon, especially for a first project 😄 Code is fairly clean and for the most part well encapsulated, you're using the ... vararg passed in by the client to namespace your functions etc. So lots of things already done well.

The saved variables thing catches a lot of people out in their first projects, because it's natural to assume they are something unique to your addon. Unfortunately they are not they are just Lua arrays / tables no different to any other array / table defined in code.

You can do /dump MiniMapDB in-game and it will print that array contents to chat. You can do /run MiniMapDB={} and it will reset it. Other addons can accidentally do that if they are leaking a MiniMapDB variable or using it as a name for their own minimap settings. In the latter case, if two addons are using the same name to reference their minimap settings, the one that loads last (alphabetic order) will overwrite the first one, or more likely try to use the other addon's settings

So like you do in your actual code trying to encapsulate variables and functions and keep them local to your addon the same applies to saved variable table names.

Since you cannot avoid putting them in the global namespace that all addons share, the best you can do is try give them a name that is likely to be unique (<addonname>_suffix is a good naming convention for saved variables.

[Road-block]

I'll try send a pull request so you can see it in practice.

Edit: I want to say this is nothing urgent, the addon works fine as is. It's more about future-proofing than anything.

[1onar]

That's a hard one but i am mostly done.

[Road-block]

Something I would like to point out if it helps. You are already using this facility but not fully 😺

The wow client passes into every Lua file referenced in the .toc of an addon a vararg ... First argument is the addon folder name (string) and second argument is a Lua table (array) that is shared between all the addon files without being global.

It is a - particular to WoW - "my addon scope".

local addonName, addon = ...

on top of your Lua files lets you pass around variables (including methods, other tables etc) without having to put them in the global namespace that all addons share.

So let's say you have an inCombat boolean that you want to use in two of your addon files but does not need to persist in saved variables.

addon.inCombat = true

in file1.lua is perfectly accessible in file2.lua so long as it also has assigned the local addonName, addon = ... at the top.