Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: multi file addon, addon ns #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grabthar
Copy link

  • support for multiple file addons
  • support (client) ability to include other file in the same addon

- support for multiple file addons
- support (client) ability to include other file in the same addon
@yuanf225
Copy link

nice job

@Rochet2
Copy link
Owner

Rochet2 commented Sep 10, 2022

Hmm. There is no multi file addon support added other than the namespaces?
Or am I missing something?

The addon namespaces seems to be something that one can create already by doing MyAddon = MyAddon or {} as a part of the addon code. Also here is a bit more context for further reference about addon namespaces https://stackoverflow.com/questions/13565828/lua-wow-and

I guess this adds some convenience, but not quite sure its worth sacrificing backwards compatibility. Maybe it could be done differently while achieving the same results. For example, maybe the namespace does not need to be defined by the system.
For example, maybe AIO can expose the AIO namespace table and the loaded addon name/filename similar to how you have done it. And then they are used to create and access a namespace without any modifications to the rest of the system. Essentially only a single function is added as well as RunAddon(name) would be modified to enable passing in the AIO.lua namespace as well as the addon name.

-- AIO.lua
function AIO.Namespace(ns_name, addon_name, aio_ns_table)
    aio_ns_table[ns_name] = aio_ns_table[ns_name] or {}
    return addon_name, aio_ns_table[ns_name]
end

-- client.lua
local AIO = AIO or require("AIO")
-- the ... dots are addon_name, aio_ns_table passed in by RunAddon
local name, namespace = AIO.Namespace("MyAddon", ...)

@grabthar
Copy link
Author

The addon namespaces seems to be something that one can create already by doing MyAddon = MyAddon or {} as a part of the addon code. Also here is a bit more context for further reference about addon namespaces

This is not the same thing, MyAddon is a global while here the namespace I use is the "AIO" addonnamespace, other addons non AIO won't be able to use this ns.
I just forward what the client offer and split that over many "addons", It also ensure/allow an addon file name is unique per addon. The idea behind thoses changes is to have: separation of concern for devs, smaller diff for clients to receive addon update, and addon with the same behavior as client addons.

-- the ... dots are addon_name, aio_ns_table passed in by RunAddon
local name, namespace = AIO.Namespace("MyAddon", ...)

This is not the standard syntax wich is:

-- the ... dots are addon_name, aio_ns_table passed in by RunAddon
local name, namespace = ...

With my solution you could for example parse a toc file to send a real addon because the client syntax is preserved. I think there's many pro/cons and many way to achieve this. I did my best to improve AIO, the choice to merge or not is up to you!

I also worked on a backport solution but this is not great, https://github.com/grabthar/ElunaAioScripts
Maybe some work on this could improve the backward compat?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants