Aller au contenu
  • J'aime 1
  • J'aime 2

Review Averages

5.0 out of 5 stars
100%
0%
0%
0%
0%

User Feedback

Vous ne pouvez donner votre avis qu'après avoir téléchargé cette ressource.


Romitou

   3 sur 3 membres a ou ont trouvé cet avis utile 3 / 3 membres

Bonjour, un add-on très bien réalisé et qui apporte de belles fonctionnalités !
Je souhaite si possible aider à l'améliorer selon mes connaissances acquises lors de la réalisation d'un add-on :

➡️ Utiliser des expressions propriétaires lorsqu'il est possible
Par exemple, pour l'expression ExprClientIP, je pense qu'il serait préférable d'utiliser une propriété, c'est-à-dire :

[the] <value> of <owner>
<owner>'s <value>

Ici, le propriétaire étant clientsocket et la valeur l'IP. Pour cela tu peux utiliser une SimplePropertyExpression ainsi que la méthode register.

➡️ Améliorer les méthodes toString
Afin de donner plus d'informations aux utilisateurs lorsque Skript utilisera cette méthode, je pense qu'il faudrait inclure les valeurs de l'expression directement dans le toString.
Par exemple avec ExprServerPort :

    private Expression<AdaptServerSocket> server;    

    # ... </>

    @Override
    public @NotNull String toString(final @Nullable Event e, final boolean debug) {
        return "get server's port";
    }
    
    # Pourrait devenir :

    @Override
    public @NotNull String toString(final @Nullable Event e, final boolean debug) {
        return "get " + server.toString(e, debug) + "'s port";
    }


➡️ Possibles problèmes avec les patterns
Par exemple avec le pattern de CondSocketConnected, il est de :

%socket%[ is|'s] connect[ed]

Les parenthèses signifient qu'il faut faire un choix entre plusieurs propositions, et les crochets représentent l'optionnalité de certaines parties de la syntaxe.
Si tu souhaites donner le choix entre is et 's aux utilisateurs mais que ces choix soient optionnels, tu devrais utiliser : [(is|'s)].

De plus, Skript gère les choix optionnels et les espaces, il n'est pas nécessaire d'inclure les espaces dans les optionnalités. Encore une fois ça n'engage que moi et ma façon de voir les choses, mais la syntaxe finale ressemblerait plutôt à :

%socket% [(is|'s)] connect[ed]

L'utilisateur sera toujours dans la possibilité d'utiliser if %socket% connected:.

Salut, merci beaucoup pour ta review.
Je viens d'ajouter des fonctionnalités (plugin messages) et en même temps, j'ai pris en compte tes deux derniers points et je les ai intégrés.
Quant au premier, j'ai pas encore compris cette classe donc je ne l'ai pas mise. Je le ferai sûrement un après midi.

Merci !


Lien vers l’avis



Retour utilisateur

×
×
  • Créer...

Information importante

Nous avons placé des cookies sur votre appareil pour aider à améliorer ce site. Vous pouvez choisir d’ajuster vos paramètres de cookie, sinon nous supposerons que vous êtes d’accord pour continuer.