-
Notifications
You must be signed in to change notification settings - Fork 159
Storage Module Update + Action History (.json use / no dependencies) #910
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
base: master
Are you sure you want to change the base?
Conversation
… dependencies using a .json file
…story display format
…(BAN, KICK, WARN, UNBAN) across the action history
Very open to suggestions / additions, somewhat in early stages |
…ntly, regardless of action history setting
…ved clarity and performance
@Blumlaut Is this use of storage.lua more along the lines of what you were thinking? |
…functions for improved organization
i'll have to review the code in detail when i have more time but glancing over it thats about what i was thinking of, i'm just wondering how to best make this work with plugins, i wonder if we should just let them overwrite the Edit: Maybe instead of letting them overwrite the storage entity introduce something like with plugins and have a storage backend convar that people can set, this would also allow us to merge additional storage backends without having to overwrite the default |
Would it be worth removing the custom banlist convar in favour of this updated storage system or should the function still check for it for backwards compatibility |
Good question, it hasn't been officially supported (or documented..) for years now, i'm not sure if it works any more even. I'd probably drop it altogether and force people to migrate to the new system. |
…ction history management
…methods and action logging
Ban system completely overhauled with new system, commented at points throughout banlist.lua just to make my ideas clear. At the moment, no old code is deleted, rather commented out to ensure nothing is missed. Small thing with this storage module, the performBanlistUpgrades() might not work as expected, I'll have to try it out once I get back to my PC |
Also, if anyone is a good discord.js developer, feel free to try and write some commands to view action history. Alternatively, I abandon discord.py for the week and learn djs to make it 😭 |
i can probably write some things for it whenever i have the time, i need to remove the configure command and document that change anyhow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been a second since I've looked at this PR, I think I've cleared up most of the concerns, feel free to look it over and get back to me with any concerns
refactor(fxmanifest): remove unused default storage backend option
fix(banlist): update function name for consistency in banlist retrieval refactor(storage): rename local variable for clarity in action retrieval
fix(server): update getBanlist function name for consistency refactor(server): rename local variable for clarity in getAction function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIXED
just glanced over the code but it looks like there's still no mechanism to have storage backends perform migrations, what are you thinking here? should the storage backends take care of migrations upon being started or should EA "trigger" them when it deems them necessary? Thinking about it, the first option may make more sense, although we should probably still include a function call for it purely so people dont go around trying to hook a different function or whatever to make it work instead of just doing a self-calling function. |
Future Considerations: