Implement commands system #40
Conversation
Closes alveusgg#32 Closes alveusgg#33 Closes alveusgg#34 Signed-off-by: flakey5 <[email protected]>
| const result = command.run({ | ||
| controller: this.#controller, | ||
| channel, | ||
| user, | ||
| args, | ||
| msg | ||
| }) | ||
|
|
||
| if (typeof result?.then === 'function') { | ||
| await result | ||
| } |
There was a problem hiding this comment.
Instead of this if, I think you could do await Promise.resolve(command.run(...))?
| } | ||
|
|
||
| if (permission.group && user in groupMemberships) { | ||
| const userGroup = groupMemberships[user] |
There was a problem hiding this comment.
I would think we'd want a user to be able to be in multiple groups (so we could have a group for full PTZ commands, a group for just the ptzload etc. commands, and so on)?
I think that might be an easier approach that having an intrinsic ranking to groups?
| throw new TypeError(`${file}: expected enabled to be a boolean, got ${typeof command.enabled}`) | ||
| } | ||
|
|
||
| if (typeof command.permission !== 'undefined' && typeof command.permission === 'object') { |
There was a problem hiding this comment.
| if (typeof command.permission !== 'undefined' && typeof command.permission === 'object') { | |
| if (typeof command.permission !== 'undefined' && typeof command.permission !== 'object') { |
- worth also checking the structure beyond that it is an obj?
| /** | ||
| * @type {Array<import('./types.d.ts').Command>} | ||
| */ | ||
| const commands = aliases.map(alias => ({ |
There was a problem hiding this comment.
I think the command structure should have an aliases prop (during the command loading you can generate an obj/map to do quick lookups) instead of us needing to create multiple identical objects for each alias (and having a single obj per command means we could more easily use the data to power the site commands list etc.)
| @@ -496,6 +496,8 @@ class Axis { | |||
| * | |||
| * `controller.connections.cameras` is an object of Axis camera connections | |||
| * | |||
| * @typedef {Record<typeof config.axisCameras[number], Axis>} CamerasConnection | |||
There was a problem hiding this comment.
A typedef inside the jsdoc for a func seems wrong?
|
@dansza1 @wazix11 @spacevoyager03 @pjeweb would appreciate y'all taking a look at this draft as well and hearing any thoughts you have on it in terms of direction |
I think generally this is a good approach. I'm not the biggest fan of classes in JS but that is besides the point. I have not thought about the permissions in depth yet, even though that seems to be one of the bigger points here. Maybe someone else can think that through ;) There are a bunch of commands with shared code, sometimes only changing one var, like customcamstl, …tr etc. I think it would make sense to allow them as aliases of one command and pass the command name as a CommandArg. Also I would probably move the alias mapping in to the command manager and define them in the exported config as an array or something. |
As titled and described in the issues linked.
Opening this as a draft just to get feedback on the approach here before moving forward.
What's currently here:
ptzplayaudioandptzstopaudiocommands for demonstration purposesFor the permission system, it's pretty similar to what's there now with two main differences:
indexOf)This does allow for more granular access to commands, but, it makes giving someone access to say all the
ptzcommands w/o adding them to a group kinda annoying since you'd need to add them to each of the command's permissions individually. A better approach to this might be having permission "nodes" similar to how Minecraft does it (and also might be what #34 is talking about here:?). Then, going back to the example of giving someone access to all the
ptzcommands, we can just make all theptzcommands take the permission nodeptz-bla-bla-bla-something. Then inconfig.jswe define which groups and individual users have a permission node. If a user has that permission node or the group they're in has that node, then they can run the command.Closes #32
Closes #33
Closes #34