feat/fix: addon permissions/updating outdated function#610
feat/fix: addon permissions/updating outdated function#610RickyB505 wants to merge 3 commits intoTomGrobbe:developmentfrom
Conversation
added individual permissions to addon vehicles/weapons/peds, and added a whitelist to regular vehicles/ped. changed (uint)GetHashKey() to Game.GenerateHashASCII() for better optimization
| { | ||
| public static class AddonPermissionsManager | ||
| { | ||
| public static List<string> Permission = new() |
There was a problem hiding this comment.
One issue I see that might come up is the names of these permissions. Other than that LGTM
There was a problem hiding this comment.
shouldn't be any issue none of them collide with default permissions
| } | ||
| else | ||
| { | ||
| // Setup addon permissions |
There was a problem hiding this comment.
Nitpick: This comment should be removed or actualy code should be added ;)
There was a problem hiding this comment.
oh that i forgot to remove oops
| #region Utilities | ||
| private void SetupAddonPerms() | ||
| { | ||
| Dictionary<string, List<string>> addons = Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, List<string>>>(LoadResourceFile(GetCurrentResourceName(), "config/addons.json") ?? "{}"); |
There was a problem hiding this comment.
It would be best if the config/addons file was only loaded once, instead of multiple times and deserialized multiple times. Somewhere above in the code it's already being loaded once. Please make a field on the class that can be reused.
Also, not sure if it matters, but maybe using Path.Combine would be preferred for Linux/Windows differences. Although I'm not sure how LoadResourceFile even deals with this.
There was a problem hiding this comment.
this is basically the same method used through out the menu i just shortened it to 1 line from 2
There was a problem hiding this comment.
and it should only be called once on restart/startup its why i didn't make it a field but i can adjust that for you
| addonPermissions.Add("add_ace builtin.everyone \"" + AddonPermissionsManager.GetAceName(permission) + "\" allow"); | ||
| } | ||
|
|
||
| System.IO.File.WriteAllLines(GetResourcePath(GetCurrentResourceName()) + "/AddonPermissionsTemplate.cfg", addonPermissions.ToArray()); |
There was a problem hiding this comment.
What is this for? And does it need to happen every time?
And maybe since we're using LoadResourceFile, this should be kept as SaveResourceFile?
There was a problem hiding this comment.
If you're going to make a template file, I'd suggest that you also include some information at the top of the file explaining what it is, what people should do with it, and that they probably shouldn't touch the file itself because that seems to get overwritten each time. Just so we don't confuse people who attempt to edit it and then execute it or something.
There was a problem hiding this comment.
i didn't use SaveResourceFile because this preformatted it into a clean file but if you want me to replace it i can, also good call i should have added a comment to the top ill change that now
There was a problem hiding this comment.
Discussed in Discord, fine to keep like this. Only thing I'm worried about is the hardcoded /, I'd rather you use Path.Combine. Just cause servers can be on linux and windows.
3 participants
added individual permissions to addon vehicles/weapons/peds, and added a whitelist to regular vehicles/ped. changed (uint)GetHashKey() to Game.GenerateHashASCII() for better optimization