Sleitnick / AeroGameFramework

AeroGameFramework is a Roblox game framework that makes development easy and fun. The framework is designed to simplify the communication between modules and seamlessly bridge the gap between the server and client.
https://sleitnick.github.io/AeroGameFramework/
MIT License
215 stars 57 forks source link

Ability to disable WrapModule on modules #187

Closed OverHash closed 4 years ago

OverHash commented 4 years ago

This is particularly useful for shared modules, which return simply a table.

I was trying to loop over this table, and access some properties - but there is an _events injected which causes my loop to error. To get around this, I had to make another scope inside my table, so that instead of

return {
    ...
}

it was

return {
    data = {
        ...
    }
}

This can be annoying, and there should be a way to set a key such as __aeroDisableWrap to disable this behavior.

OttoHatt commented 4 years ago

I have run into this issue a few times, and agree that we need a way to disable any modifications/wrapping on a table. However, I could only see this being useful if __aeroDisableWrap doesn't exist in the required table - otherwise there is still an unexpected key in the table when looping through it. This is a similar issue for all Aero flags.

I think that the system could be revamped to the following, using an optional accompanying module:

-- Client/Shared/MyData.lua
return {
    Foo = "Bar";
}

-- Client/Shared/MyData.settings.lua
return {
    __aeroDisableWrap = true;
    __aeroPreventInit = true;
    __aeroPreventStart = true;
    ...
}

As MyData.settings.lua disables wrapping, MyData.lua will have no modifications made to it by Aero.

When requiring a module, Aero would first check for a settings module. Then it will combine the flags of the settings module and main module together, then act accordingly on them. An edge case where a settings module's flags overwrites a main module's flags must be accounted for.

This could be expanded to directory-wide settings, letting you assign readonly settings to a whole folder of data-only modules:

--Client/Shared/ShopData/.settings.lua
-- A different name for the settings module may be better, i.e. thisdirectory.settings.lua
return {
    __aeroDisableWrap = true;
}

-- Client/Shared/ShopData/Foo.lua
-- Won't wrap
return {
    ...
}

-- Client/Shared/ShopData/Bar.lua
-- Won't wrap
return {
    ...
}

-- Client/Shared/Baz.lua
-- Will wrap
return {
    ...
}

The Aero plugin could be used to create and edit settings modules automatically. Support for the old method of putting flags inside the table will still be supported.

Sleitnick commented 4 years ago

Yeah this would be good. I think the simplest way would be something like a __aeroIgnore = true flag. Essentially that would just treat the module as a plain-old module and not include it within the Aero system.

Of course, the drawback is that the module won't be able to tie into the system directly anymore. But that's probably fine.

OverHash commented 4 years ago

Well, the goal would be that other files can access the module, but the module that is not areo wrapped cannot access others.

Sleitnick commented 4 years ago

@OverHash Yup, correct, it would still be accessible from others

Sleitnick commented 4 years ago

It would probably make most sense that this feature would only be allowed in either the Modules folders or Shared folder.

OverHash commented 4 years ago

The file.settings module option is pretty neat. Can't think of anything more optimal.

OttoHatt commented 4 years ago

I think you should be able to prevent wrapping without adding the __aeroIgnore flag. It's bad practise to modify libraries and discouraged by Roblox (loading Roact with Aero errors as its metatable throws when you index nil or modify it). Could become a problem if packages are implemented as script instances that you require but can't modify.

I think that the .settings files or a read-only folder would be a better option.

OverHash commented 4 years ago

I think you should be able to prevent wrapping without adding a flag. It's bad practise to modify libraries and discouraged by Roblox (loading Roact with Aero errors as its metatable throws when you index nil or modify it). Could become a problem if packages are implemented as script instances that you require but can't modify.

This is why it would be a separate file, as to not interrupt with the source of the original.

Sleitnick commented 4 years ago

Could also do both. I think adding a .settings file is a bit clunky. But I see the use-case for third-party modules.

Sleitnick commented 4 years ago

This will be provided in v1.7.0. Other flags will be included too.

As an example, if there's a module called "MyMod" then it can have an accompanying module called "MyMod.settings" with a structure as such:

return {
    Order = nil;
    PreventInit = false;
    PreventStart = false;
    Standalone = false;
}

The Standalone flag will dictate whether or not a module gets the AGF injection.


For instance, DataStore2 could have a DataStore2.settings file with the following config:

return {
    Standalone = true;
}