Proposition d'écriture de commande

[Sebastien-Ahkrin]

Ce que nous avons imaginé

Pour le moment, nous pensons faire une implémentation de la création de commande de cette façon:

const options: ClientOptions = {
  prefix: '!',
  commands: join(__dirname, 'commands')
}

const client = new Client(process.env.DISCORD_TOKEN as string, options);

import Command from "../../package/commands/command";

const command = new Command<[string, number]>('hello', async (args, message) => {
  await message.reply('Hello, World!');
});

export default command;

La commande serait donc utilisée de cette façon: !hello arg1 arg2 Les deux arguments (arg1, et arg2) seraient du type générique définie plus haut ([string, number]), les arguments ne seraient donc pas nommée, mais cette façon de faire n'est pas exclue !hello --arg1 Hello --arg2 123, de plus, il doit être possible d'avoir une chaine de plusieurs mots en arguments.

Toutes les commandes seraient donc chargés par le dossier qu'on donnerais dans la config (ClientOptions).

Que pensez-vous de tout cela ? Avez-vous des idées ? des choses à ajouter ?

[Mesteery]

import Command from '../../package/commands/command';
import isOwner from 'quelquepart'

class HelloCommand extends Command<[string, number]> {

  constructor () {
    super({
      name: 'hello',
      aliases: ['salut', 'bonjour'],
      permissions: {
        client: ['ADMINISTRATOR'], // les permissions requises pour le bot
        user: ['MANAGE_MESSAGES', 'BAN_MEMBERS'], // les permissions requises pour l'utilisateur 
        // et/ou
        customCheck: (client, user) => client.ownerID === user.id /* ou appeler une méthode privé (ou une fonction) */ isOwner
      },
      throttling: {
        duration: 10000,
        usages: 2 // 1 par défaut
      } // l'utilisateur peut seulement exécuter 2 fois cette commande en 10 secondes
    });
  }

  async run (message, args) {
    await message.reply('hello !');
  }

}
export default HelloCommand;

Je préfère faire les commandes dans des classes, c'est bien plus pratique Et je trouve le système d'arguments limités (en utilisant la généricité), c'est mieux de pouvoir déclarer les arguments avec des types, des noms (pour y accéder comme args.example), des valeurs par défauts, des vérificateurs de types custom (par exemple avec une fonction fléché, une méthode privé ou une fonction).

[
  {
    name: 'target',
    description: 'La personne que vous voulez saluer', // pratique pour l'aide
    example: '767019913465430048', // ou 'ESC Community#3172', ^
    type: ?, // j'ai pas trop d'idée de comment définir le type des arguments
    default: null // = argument optionnel qui vaut par défaut null
  },
  // ...
]

Pouvoir ajouter des options pour les permissions, autoriser ou refuser l'accès à une commande.

On peut aussi un peu s'inspirer de certains frameworks déjà existants (comme discord-akairo, discord.js-commando, Klasa, YAMDBF, DiscordThingy ou même d'escape)

[Xstoudi]

Je préfère faire les commandes dans des classes, c'est bien plus pratique

Peux-tu préciser en quoi cela est-il plus pratique ?

[Mesteery]

Pour moi c'est plus pratique, pour plusieurs raisons :

• permet de personnaliser la commande facilement (comme dans l'exemple)
• on peut avoir des méthodes pour plusieurs choses (des méthodes "utilitaires" qu'on peut utiliser dans le run, des méthodes qui sont appelées par le command handler, ...)

Il y a sûrement d'autres raisons, mais celles-ci sont celles qui me viennent en premier à l'esprit

[Xstoudi]

• Rien de ce qui n'est fait dans ton exemple n'est pas reproductible via une simple fonction.
• Rien n'empêche ta fonction handler exportée et d'avoir d'autres fonctions non exportées aside.

Vraiment, je ne vois toujours pas l'intérêt.

[Mesteery]

Tu verras bien qu'une fonction suffit pas

[DraftProducts]

Up ! Yes, pour le coup pour avoir bosser sur un bot discord depuis maintenant 2 ans @Xstoudi (draftbot), j'étais au début sur des fonctions handler exportés (il y a 2 ans) et je t'assures que je suis pas resté longtemps là dessus. Je rejoint @Mesteery sur ce point, le fait d'être sur une class te facilitera grandement la tache !

[Xstoudi]

L'argument d'autorité ne fait pas tellement avancer la question: soit on a besoin de la classe, soit ce n'est pas le cas. Là ça ferait bien avancer les choses de savoir exactement ce qui est facilité par l'utilisation de classe plutôt que d'une fonction.

[DraftProducts]

Principalement la maintenabilité des commandes, dans mon cas j'en avait de plus en plus. Derrière ça te permet d'éviter le duplicata et d'avoir différents types de fonctionnements, étant donné que tu créé une class qui extends d'une base tu peux formater des données en entrée comme tu l'entends. Ensuite tu as tout l'aspect des options, qui devient beaucoup plus simple a gérer lorsqu'il te suffit juste de les définir dans ton constructeur pour qu'elles ailles override celles de ta principal. Ensuite rien ne t'empêche d'avoir des surcouches pour certaines commandes: dans le cas où tu as un type de commande qui ne fait que fetch une api et afficher des résultats, tu pourrais avoir une classe qui embarque tous ses outils et son formatage. Pour être honnête avec toi, si vous n'avez que 3 ou 4 commandes c'est full useless mais au contraire si tu vises un truc complet ça deviendra vite rébarbatif au niveau du code si tu fais de simples export de données. Voici quelques libs et bots opensource qui te permettra de voir un peu l'utilité que ça peut avoir:

• https://github.com/SwitchbladeBot/switchblade/blob/dev/src/commands/misc/flathub.js
• https://github.com/discordjs/Commando/blob/master/src/commands/util/ping.js

Du coté de Switchblade tu as quelque chose qui rejoint un peu l'idée des surcouches avec les "types de commandes" tandis qu'avec discord-commando tu as quelque chose de plus léger dans le même type que ce qu'à proposé @Mesteery.

De mon coté sur DraftBot j'ai quelque chose d'encore plus que Switchblade avec des types de commandes dédiés à leur utilisation (modération, configuration, gifs d'emotions ect).

[Xstoudi]

Pour être honnête avec toi, si vous n'avez que 3 ou 4 commandes c'est full useless

Merci bien !

[Mesteery]

Ça m'étonnerai qu'il y ai seulement 3-4 commandes

[Xstoudi]

Pour l'instant y a surtout zéro issue labelisée "idée", donc l'estimation me semble pas déconnante ^^

[Mesteery]

Oui je suis d'accord là dessus avec toi, mais 3-4 commandes c'est très peu pour un bot, et c'est peu probable d'avoir seulement 4 commandes. Les commandes s'ajouteront au fur et à mesure, donc je ne pense pas qu'attendre d'avoir plus de 4 commandes pour modifier le framework et accepter les classes soit une bonne idée. Après j'ai envie de dire, si y aura vraiment seulement 3-5 commandes simples : pourquoi pas

[Mesteery]

J'ai une autre proposition : le framework peut accepter les fonctions et les classes Les commandes très simples utilisent les fonctions. Et les autres utilisent les classes.

Après, c'est peut-être pas la meilleure proposition (je le pense aussi).

[DraftProducts]

Bof, si tu décides de suivre un paterne, tu le suis, ça risque de devenir vite le bordel si on switch de l'un à l'autre sans cesse. Après tu peux obtenir un résultat similaire si tu export une fonction options et une option run dans ta commande et gérer tes options avant l'exécution de la commande. Si le but c'est que ce soit des commandes simples sans vraiment de configuration, de dépendances avec l'auteur de la commande et d'autres éléments qui changeraient le comportement de la commande c'est kiff kiff.

[DeltaEvo]

Malgré les nouveaux arguments je reste de l'avis de @Xstoudi

* Rien de ce qui n'est fait dans ton exemple n'est pas reproductible via une simple fonction.

* Rien n'empêche ta fonction handler exportée et d'avoir d'autres fonctions non exportées aside.

Vraiment, je ne vois toujours pas l'intérêt.

Je ne comprend pas pourquoi utiliser une classe alors que tu va avoir qu'une seule instanciation de cette derniere, quel est l'interet de la classe ? Pourquoi ne pas seulement exporter un objet avec des proprietes pour les metadata de ta commande si besoin (arguments, permissions, ...) et pour l'handler de la commande ?

Je ne vois pas aussi l'interet de l'heritage dans les exemples quand cet heritage ne sert qu'a ajouter une abstraction (par exemple le SearchCommand de switchblade) quand tu aurais juste pu faire de la composition et extraire cette abstraction et la rendre plus generique et reutilisable dans autre chose que ta commande

[Mesteery]

Avec une classe ça serai plus simple d'accéder à Bot (qui serait attribué au moment de l'instanciation)

[DeltaEvo]

Tu peux tres bien faire en sorte que tes commandes soient definies par une fonction qui prend ton bot en parametre et te retoure un object ou une fonction, pas besoin d'un constructeur et d'une instanciation pour ça Ou exporter l'instance de Bot et l'importer dans tes modules

[Mesteery]

Tu peux tres bien faire en sorte que tes commandes soient definies par une fonction qui prend ton bot en parametre et te retoure un object ou une fonction, pas besoin d'un constructeur et d'une instanciation pour ça

Oui je sais, mais le Bot peut être utilisé partout et pas seulement dans une fonction, donc je trouve pas ça une bonne idée, de devoir faire passer Bot à chaque fonction ou à chaque "partie" ayant besoin de Bot

[DeltaEvo]

Oui je sais, mais le Bot peut être utilisé partout et pas seulement dans une fonction, donc je trouve > pas ça une bonne idée, de devoir faire passer Bot à chaque fonction ou à chaque "partie" ayant besoin de Bot

Des deux propositions que j'ai faite aucune n'implique de devoir faire passer Bot a chaque fonction

[Mesteery]

Ah oui, j'ai mal lu.

[DeltaEvo]

Exactement ce que t'as partager pour le premier cas :)

fonction qui prend ton bot en parametre

```ts export default function helloCommand (bot: Bot): Command { return { name: 'hello', aliases: ['salut', 'bonjour'], args: [ { name: 'target', description: 'La personne que vous voulez saluer', // pratique pour l'aide example: '767019913465430048', // ou 'ESC Community#3172', ^ type: ?, // j'ai pas trop d'idée de comment définir le type des arguments default: null // = argument optionnel qui vaut par défaut null }, // ... ], permissions: { client: ['ADMINISTRATOR'], // les permissions requises pour le bot user: ['MANAGE_MESSAGES', 'BAN_MEMBERS'], // les permissions requises pour l'utilisateur // ... }, throttling: { duration: 10000, usages: 2 // 1 par défaut }, // l'utilisateur peut seulement exécuter 2 fois cette commande en 10 secondes async handle (message: Message, args: /* ... */) { const helloMessage = await message.reply('hello !'); helloMessage.react('👋'); } } } ```

import de bot

```ts import { bot } from '...' export default { name: 'hello', aliases: ['salut', 'bonjour'], args: [ { name: 'target', description: 'La personne que vous voulez saluer', // pratique pour l'aide example: '767019913465430048', // ou 'ESC Community#3172', ^ type: ?, // j'ai pas trop d'idée de comment définir le type des arguments default: null // = argument optionnel qui vaut par défaut null }, // ... ], permissions: { client: ['ADMINISTRATOR'], // les permissions requises pour le bot user: ['MANAGE_MESSAGES', 'BAN_MEMBERS'], // les permissions requises pour l'utilisateur // ... }, throttling: { duration: 10000, usages: 2 // 1 par défaut }, // l'utilisateur peut seulement exécuter 2 fois cette commande en 10 secondes async handle (message: Message, args: /* ... */) { const helloMessage = await message.reply('hello !'); helloMessage.react('👋'); } } ```

[Mesteery]

Franchement, je trouve ça plutôt pas mal^^ (Y a sûrement des petites choses à ajouter/enlever/modifier/améliorer)

[targos]

Le bot pourrait aussi être passé à handle. Cf l'api actuelle pour les Crons qui passe un objet contexte

[Mesteery]

Il est déjà passer à la fonction parente https://github.com/ES-Community/bot/issues/8#issuecomment-725642477

[Xstoudi]

Il est déjà passer à la fonction parente #8 (comment)

Oui, et l'intérêt est nul. Je plussoie @targos sur le fait de directement le passer au handler.

[Mesteery]

Là il n'y a pas d'intérêt puisque l'exemple n'est pas complet. Mais le Bot pourrait être utilisé à l'extérieur de handle comme dans la configuration pour pouvoir faire des vérifications supplémentaires.

On peut aussi faire comme ça et modifier au besoin :

// imports...

export default {
  {
    name: 'hello',
    aliases: ['salut', 'bonjour'],
    args: [
      {
        name: 'target',
        description: 'La personne que vous voulez saluer', // pratique pour l'aide
        example: '767019913465430048', // ou 'ESC Community#3172', ^
        type: 'member', // j'ai pas trop d'idée de comment définir le type des arguments
        default: null // = argument optionnel qui vaut par défaut null
      },
      // ...
    ],
    permissions: {
      client: ['ADMINISTRATOR'], // les permissions requises pour le bot
      user: ['MANAGE_MESSAGES', 'BAN_MEMBERS'], // les permissions requises pour l'utilisateur
      // ...
    },
    throttling: {
      duration: 10000,
      usages: 2 // 1 par défaut
    } // l'utilisateur peut seulement exécuter 2 fois cette commande en 10 secondes
  },
  async handle (context, message, args: /* ... */) {
    const helloMessage = await message.channel.send(`hello ${args.target} !`);
    helloMessage.react('👋');
  }
}

[Xstoudi]

Exemple ? Je pense qu'on a dépassé le stade des suppositions. ^^

[Mesteery]

J'ai pas d'exemple en tête. (J'ai modifié mon message entre-temps)

[DeltaEvo]

Je pense que ce qui est presenté ici est trop complexe, je pense que ou on passe Bot et comment est un detail

Je pense qu'il faut faire des choses basiques et pas commencer a vouloir gerer des permissions et du throttling surtout qu'on ne sais meme pas si on en auras besoin, je pense qu'il faut faire un truc simple au debut corriger moi si je me trompe mais une commande c'est juste un message qui commence par un prefix et un nom de commande

code

```ts // hello.ts export async function hello(message: Message): Promise { const helloMessage = await message.reply('hello !'); helloMessage.react('👋'); } // index.ts const commands = new Map(); commands.set("hello", hello) client.on('message', msg => commands.forEach((command, name) => { if (msg.content.startsWith(prefix + name)) command(msg) })); ```

Et ajouter ce dont on a besoin au fur et a mesure

Par exemple pour nos commandes peux etre qu'on auras pas besoin de parser d'argument

code

```ts export async function hello(message: Message): Promise { const [command,user] = message.content.split(' ') const helloMessage = await message.reply(`hello ${user} !`); helloMessage.react('👋'); } ```

Après c'est sur que c'est bof comme parser d'argument si tu met trop d'espace ça marche pas, mais bon rien ne nous empeche d'en faire un plus poussé

code

```ts import { parse_arguments } from './arguments.ts' export async function hello(message: Message): Promise { const [user] = parse_arguments(message, [ { name: 'user', ... } ]) const helloMessage = await message.reply(`hello ${user} !`); helloMessage.react('👋'); } ```

Mais bon avec ça c'est difficile de generer un help automatiquement, c'est pas grave rien nous emepeche de le changer

code

```ts import { with_arguments } from './arguments.ts' // with_arguments peux ajouter une propriété sur la fonction pour recuperer le format at runtime export async with_arguments( function hello(message: Message, user: String): Promise { const helloMessage = await message.reply(`hello ${user} !`); helloMessage.react('👋'); }, { name: 'user', ... } ) ```

Et par exemple si on as besoin d'un throttle

code

```ts function throttle(function, wait) { // implementation custom de throttle qui evoie un message et gere les promises } export async throttle(with_arguments( function hello(message: Message, user: String): Promise { const helloMessage = await message.reply(`hello ${user} !`); helloMessage.react('👋'); }, { name: 'user', ... } )), 300) ```

Bon après ça deviens le bordel mais rien ne nous empeche de faire un helper pour render ça plus simple pour les commande plus complexe

code

```ts function command({ handler, throttling, args }) { if (args) handler = with_arguments(handler, args) if (throttling) handler = with_throttle(handler, throttling) return handler } export async command({ handler(message: Message, user: String): Promise { const helloMessage = await message.reply(`hello ${user} !`); helloMessage.react('👋'); }, args: [ { name: 'target', description: 'La personne que vous voulez saluer', // pratique pour l'aide example: '767019913465430048', // ou 'ESC Community#3172', ^ type: ?, // j'ai pas trop d'idée de comment définir le type des arguments default: null // = argument optionnel qui vaut par défaut null }, // ... ], throttling: { duration: 10000, usages: 2 // 1 par défaut } )) ```

Bon en gros ce que j'essaye de dire dans ce message qui deviens trop long c'est que je pense qu'il faudrait se mettre d'accord sur le truc le plus simple possible et voir en fonction des commandes qu'on implemente pour le refactor/ajouter des helpers pour pouvoir gerer des trucs plus avancé

Et je pense sincerement qu'on ne devrais pas avoir un gros systeme overkill qui gere tout a un seul endroit (rip les classes :kappa:) et plus decouper les implementations et faire de la composition, ce sera surement plus simple a comprendre et aussi plus simple a tester si jamais on veux le faire a l'avenir

[Mesteery]

Je suis pas trop d'accord sur certains choses, et puis l'exemple est bien chargé est complexe, c'est juste pour montrer la plus part de utilisations. Là plupart des propriétés dans l'exemple sont optionnels, et donc comme tu le dis on peut au début implémenté les commandes sans, et ajouter des choses au fur et à mesure et au besoin. Sur certains points, c'est beaucoup trop simple, et jamais ça sera maintenable sur un bot discord.

Un exemple : .../hello.ts :

// imports...

export default {
  {
    name: 'hello',
    description: 'Vous salue',
    async handle (context, message) {
      const helloMessage = await message.reply('hello !');
      helloMessage.react('👋');
    }
  }
}

Personnellement, y a pas plus simple que ça

[DeltaEvo]

Si ça

export default async function hello(message: Message) {
      const helloMessage = await message.reply('hello !');
      helloMessage.react('👋');
}

[DraftProducts]

Je penses que les choses évolueront et même si il n'est pas prévu d'en faire un outil complet, je penses néanmoins que la scalabilité de ce dernier est pas a prendre à la légère. Comme a listé @DeltaEvo, y a beaucoup d'ajouts et features possibles, qui pourront faire l'objet de PR au fur et à mesure, mais rien ne vous oblige a tout mettre en place dès le départ. Même si l'aspect des fonctions a l'air beaucoup plus simple à première vue et peut être plus modulable ça peut devenir vite le bordel. J'ai conscience que les projets sont pas du tout comparables mais je sais pas si tu te souviens @DeltaEvo quand j'avais commencé a bosser sur DraftBot, j'avais un fichier utils dans lequel je mettais tous mes "helpers" et la finalité c'est que seulement une trentaine étaient utilisés et le reste ne servait que dans des cas bien précis. A coup sûr, la faute à mon organisation, mais après un rewrite j'ai fini par garder plusieurs fichiers utilitaires et tout ce qui est utilitaires attachés à une feature en particulier à finit dans la class de la feature, au final dans le cas des commandes, il te suffit de faire évoluer ta class de base. Pareil pour tous les services. Je penses que passer par des class ça permettra vraiment de garder un code scalable sur la durée.

[Mesteery]

Si ça

export default async function hello(message: Message) {
     const helloMessage = await message.reply('hello !');
     helloMessage.react('👋');
}

Comment tu fais pour connaitre le nom de la commande ? Et l'aide ? Car ça serai intéressant d'avoir de l'aide (description, exemples, etc.) sur les commandes (commande help)

[targos]

Est-ce qu'on est d'accord de partir sur quelque chose de relativement simple avec un callback et améliorer selon les besoins? D'expérience, pour avoir travaillé sur de nombreux projets, ce n'est pas le refactoring qui est compliqué si on doit changer quelque chose en court de route. Afin de garder une certaine cohérence avec ce qui existe déjà avec Cron et le futur FormatChecker, je propose de partir sur quelque chose dans ce genre:

// imports...

export default new Command({
    enabled: true,
    name: 'hello',
    description: 'Vous salue',
    async handle (context) { // On peut aussi destructurer context directement en { message }
      const helloMessage = await context.message.reply('hello !');
      helloMessage.react('👋');
    }
});

Le new Command est pour avoir un typage de l'objet d'options. C'est plus ergonomique a mon avis que export default {} as Command ou const command: Command = {}; export default command

[Sebastien-Ahkrin]

Ok, on go comme ça, j'update ce que j'ai fait et je pr :)

[targos]

Vu que rien n'est encore implémenté, est-ce qu'on devrait en profiter pour utiliser le système de commandes de Discord?

[Mesteery]

Oui ! Il faudrait juste attendre la v13 de discord.js, ou bien utiliser la master. Je voudrais bien implémenter cette fonctionnalité au passage.

[DraftProducts]

Yes, je penses que ça serait un bon move. Je peux aider au besoin. Y'a les nouveaux boutons & sélecteurs qui pourraient éventuellement faire évoluer certains comportements sur la communauté.

[Purexo]

Je propose de clore cette issue, et ce pencher sur le sujet le jour ou on aura une idée de commande à ajouter au bot. 2ans après l'ouverture de l'issue, malgré une PR pour intégrer le système de Command de Discord maintenant legacy, on ne l'a pas mergé et toujours pas de besoin de commande pour notre Bot 😅

[Mesteery]

J'ai pas suivi les news de Discord. Les slashs commands sont devenues legacy ?

[Purexo]

J'ai pas dig en profondeur, mais si j'ai bien suivis ça ce fait autrement les commandes de bot maintenant

[DraftProducts]

Yep, elles sont fortement conseillées et obligatoires pour les bots publics. C'est assez intéressant en terme d'UX, niveau DX le plus simple, c'est de récupérer les data des commandes (nom, desc, permissions, sous commandes, options ect...) et de les envoyer à Discord au lancement du bot.

[Mesteery]

Donc elles (slashs commands) ne sont pas legacy comme le dit Purexo ?

[Xstoudi]

Non justement c'est les slash commands qui sont le standard et j'avais pas vu que c'est ce que tu avais utilisé dans ton draft.

Maintenant :

• la proposition d'implémenter des commandes fête bientôt ses deux ans
• le draft de Mesteery a fêté son année en juillet
• y a un eu désaccord sur l'implémentation de Mesteery
• Sébastien voulait faire une autre PR, mais ça a visiblement été aborté

Mesteery est-ce que tu veux bosser sur une implémentation sans classe de ta PR, ou est-ce quelqu'un d'autre veut s'y coller ?

L'idée de base c'était juste de mettre un coup de ménage sur le dépôt et de dégager ce qui ne fait plus de sens donc cette issue peut rester ouverte, mais Mesteery : comptes-tu continuer à travailler sur ta PR en draft ou est-ce qu'on peut l'archiver ?

[Mesteery]

Je comptais bien travailler dessus mais je n'ai plus eu de review depuis, c'est la raison pour laquelle je l'ai mise en draft. (Btw y a pas de classes dans ma PR)