Bonnes pratiques

RedRelay

Bonjour,

Dans le monde du dev, il existe des conventions qui permettent a plusieurs dev de travailler ensemble plus facilement.
Dans le monde Java, on peu parler par exemple de la convention de nommage, que je vous invite a lire ici si ce n’est pas déjà fait.

Minecraft Forge est un framework assez libre, ou l’on peut organiser son code fonctionnel un peu comme on veut.
Le but de ce post, est de proposer une liste de règles afin d’uniformiser nos mods.

Ces règles n’affecterons pas les performances dans la plus part des cas.

Les 2 gros avantages :

Si quelqu’un reprend votre mod, il ne sera pas dépaysé par sa nouvelle structure.
Travailler ensemble sur un même mod va devenir plus simple, donc la productivité sera accrue.

Je vais lister l’ensemble des règles que je préconise, elles sont ouvertes a la discutions. Si vous avez aussi des règles a proposer n’hésitez pas. Le but n’est pas d’imposer un ensemble de règle, mais d’arriver le plus possible vers un consensus.

Convention

~~La classe principale de votre mod ne doit servir qu’a initialiser les proxies, les méthodes @EventHandler doivent appeler une méthode délégué au proxy. (semantique)~~
Les event handlers doivent être regroupé par fonctionnalité.
Les event handlers ne servent qu’a catcher un event, le handler appel ensuite une methode délégué par une autre classe.
Si la methode délégué est serverside only ou client side only, la vérification peut se faire dans le event handler, mais le event handler ne réalise aucun traitement fonctionnel. (monté de version simplifié)
IMessageHandler doivent être des classes interne des IMessage (Diesieben)
~~Vos blocks doivent étendre une classe générique propre a votre mod, idem pour les items. (Wuppy)~~
Les interfaces doivent commencer par un I majuscule (A la base c’est une convention .NET, mais repris par pas mal de dev car plus visuelle)
Les enums doivent commencer par Enum

J’hésite a ajouter le point suivant, car ça me semble moins général que tout le reste :

La configuration de votre mod doit se trouver dans une classe a part, le mieux étant que cette classe implémente une interface.

Pour le dernier point je m’explique :
Imaginons que vous avez un mod avec un nom custom, vous souhaitez charger ce nom custom. Si vous charger se nom directement dans le code fonctionnel, d’une part vous ajouter de la complexité, d’autre part si vous avez besoin de charger se nom plusieurs fois vous dupliquer le code donc votre code est moins maintenable. Une bonne pratique consiste a créer une interface et son implementation qui possèderont par exemple une methode getCustomName(). L’avantage de l’interface c’est que si vos avez enregistrer la config dans un fichier .properties et que vous souhaitez passer a un fichier xml ou json ou autre, vous n’avez qu’a fournir une autre implementation de l’interface.

Cette liste n’est pas fixe, dites moi ce que vous en pensez

DiabolicaTrix

C’est un très bonne idée, par contre j’ai 1 ou 2 points que je ne suis pas certain.

@Blackout:

La classe principale de votre mod ne doit servir qu’a initialiser les proxies, les méthodes @EventHandler doivent appeler une méthode délégué au proxy. (semantique)

Est-ce que ça inclus le PreInit, Init, PostInit, ServerStarting, etc ou seulement les “MinecraftForge.EVENT_BUS.register” et “FMLCommonHandler.instance().bus().register”?

@Blackout:

Les event handlers ne servent qu’a catcher un event, le handler appel ensuite une methode délégué par une autre classe.
Si la methode délégué est serverside only ou client side only, la vérification peut se faire dans le event handler, mais le event handler ne réalise aucun traitement fonctionnel. (monté de version simplifié

Tu veux bien dire qu’on peut mettre le “SideOnly(Side.CLIENT)” sur l’event handler mais qu’il ne doit pas le traité?

Sinon, je crois que je respecte seulement 2 points, le I devant les interfaces et le IMessageHandler. ^^

RedRelay

@‘DiabolicaTrix’:

Est-ce que ça inclus le PreInit, Init, PostInit, ServerStarting, etc ou seulement les “MinecraftForge.EVENT_BUS.register” et “FMLCommonHandler.instance().bus().register”?

Tu veux bien dire qu’on peut mettre le “SideOnly(Side.CLIENT)” sur l’event handler mais qu’il ne doit pas le traité?

Juste les methodes préfixé de l’annotation @EventHandler, mais normalement tu ne dois pas avoir de @SubscribeEvent dans ta classe principale. C’est sous entendu. Les @SubscribeEvent doivent être dans une classe a part, dans un event handler.

Tu peux mettre un SideOnly sur l’event handler, mais ta methode ne dois pas faire de traitement fonctionnel.
En gros tu as


@SideOnly(Side.CLIENT)
@SubscribeEvent
public void taMethodeDansTonEventHandler(TonEvent event) {

    tonObject.onEvent(event);

}

et c’est “tonObject” qui s’occupe des traitements fonctionnels.

Phenix246

@‘Blackout’:

La classe principale de votre mod ne doit servir qu’a initialiser les proxies, les méthodes @EventHandler doivent appeler une méthode délégué au proxy. (semantique)

Sincèrement je n’utilise des proxy principalement que lorsque j’appelle des méthodes coté client, quand c’est coté serveur (commun) j’appelle les méthode directement

@‘Blackout’:

Les event handlers doivent être regroupé par fonctionnalité.
Les event handlers ne servent qu’a catcher un event, le handler appel ensuite une méthode délégué par une autre classe.
Si la methode délégué est serverside only ou client side only, la vérification peut se faire dans le event handler, mais le event handler ne réalise aucun traitement fonctionnel. (monté de version simplifié)

Il me parait logique de regrouper dans une même classe tout les Events relatif au fonctionnement par exemple pour l’initialisation et la sauvegarde des playerProps. Néanmoins aller jusqu’à créer des classe avec des méthode appelées lorsque ces events sont déclenchés, je ne trouve pas ça logique, tu appelles des méthodes en cascade ce qui n’est pas considéré comme une bonne manière de développer en général.

@‘Blackout’:

IMessageHandler doivent être des classes interne des IMessage %(#336633)[(Diesieben)

Là je suis d’accord avec toi, on a ainsi le code du packet (IMessage) et son process (IMessageHandler) dans le même fichier, Et on a pas à chercher 5 minutes où le packet est traité.

@‘Blackout’:

Vos blocks doivent étendre une classe générique propre a votre mod, idem pour les items. (Wuppy)]

Bonne idée dans le cas où les blocks sont simple, c’est à dire hérité de la classe Block, néanmoins dans le cas où les block sont hérité d’un block plus particulier, par exemple les escaliers, la classe générique n’a presque aucun intérêt si ce n’est préciser la creativeTab;

@‘Blackout’:

Les interfaces doivent commencer par un I majuscule (A la base c’est une convention .NET, mais repris par pas mal de dev car plus visuelle) (monté de version simplifié)

Convention de nommage utilisé par Minecraft et en Java en général, Il en va de même pour les Enumérations.

@‘Blackout’:

La configuration de votre mod doit se trouver dans une classe a part, le mieux étant que cette classe implémente une interface.

Il est préférable de faire la configuration d’un mod dans un fichier à part, tout comme de déclarer les variables global de ton mod (nom, version, variable de configuration, etc) dans une classe à part.

DiabolicaTrix

@‘Blackout’:

@‘DiabolicaTrix’:

Est-ce que ça inclus le PreInit, Init, PostInit, ServerStarting, etc ou seulement les “MinecraftForge.EVENT_BUS.register” et “FMLCommonHandler.instance().bus().register”?

Tu veux bien dire qu’on peut mettre le “SideOnly(Side.CLIENT)” sur l’event handler mais qu’il ne doit pas le traité?

Juste les methodes préfixé de l’annotation @EventHandler, mais normalement tu ne dois pas avoir de @SubscribeEvent dans ta classe principale. C’est sous entendu. Les @SubscribeEvent doivent être dans une classe a part, dans un event handler

J’avais confondu @EventHandler et @SubscribeEvent

@'Pheni:

@‘Blackout’:

Les interfaces doivent commencer par un I majuscule (A la base c’est une convention .NET, mais repris par pas mal de dev car plus visuelle) (monté de version simplifié)

Convention de nommage utilisé par Minecraft et en Java en général, Il en va de même pour les Enumérations.

Certaines interfaces de Minecraft n’ont pas de I devant, mais je suis d’accord. Les interfaces devraient commencer par un I même si Eclipse les identifies.

jglrxavpok

@‘Blackout’:

Les interfaces doivent commencer par un I majuscule (A la base c’est une convention .NET, mais repris par pas mal de dev car plus visuelle) (monté de version simplifié)

J’aimerai ajouter qu’il faudrait faire la même chose pour les enums et ajouter ‘Enum’ devant le nom de la classe, par exemple: EnumPlayerTypes

Phenix246

Il faut savoir qu’ eclipse comme le compilateur se moquent d’où sont rangées nos classes et leur nom. Le but des conventions est qu’un développeur puisse en comprendre un autre

robin4002

@‘Phenix246’:

[texte plus haut, cliquez pour le voir]

Entièrement d’accord avec tous les points.

RedRelay

Sincèrement je n’utilise des proxy principalement que lorsque j’appelle des méthodes coté client, quand c’est coté serveur (commun) j’appelle les méthode directement

Ce qui n’est pas correct sémantiquement. Ta classe principale est un wrapper, elle n’a normalement aucun autre role que d’initialiser ton mod. Les proxies par contre sont les modules de ton mod, le module commun et le module client. A quoi servent les proxy sinon ? Dans ton cas, a quoi sert ton common proxy ? Pourquoi ne pas faire mettre un attribut a null dans ta classe principale que tu instancie uniquement si tu es en client-side dans ton @EventHandler init() ?

Il me parait logique de regrouper dans une même classe tout les Events relatif au fonctionnement par exemple pour l’initialisation et la sauvegarde des playerProps. Néanmoins aller jusqu’à créer des classe avec des méthode appelées lorsque ces events sont déclenchés, je ne trouve pas ça logique, tu appelles des méthodes en cascade ce qui n’est pas considéré comme une bonne manière de développer en général.

Tu trouve par exemple en entreprise des architectures n-tiers ou chaque tiers font un appel délégué a la couche suivante. (Présentation - Metier - DAO - BDD), tu retrouves le même shémas dans la modèle de communication OSI avec ses 8 couches. Un design pattern du nom de business delegate se base la dessus. Donc si c’était considéré comme une mauvaise manière de développer, ça n’existerait pas ^^ L’utilisation d’appel en cascade doit être justifié. Ici, c’est la séparation de la couche technique avec la couche fonctionnelle. Le Handler doit se charger d’effectuer les vérifications sur la side etc et servir de filtre. Par exemple, imaginons une classe qui a besoin d’effectuer une action lorsque le joueur fait un clique droit. La classe va posséder une methode onRightClick(…) et le handler va s’occupé de géré la side et de filtré l’event PlayerInteractEvent pour que la methode onRightClick soit appelé uniquement lorsque le joueur fait un clique droit. Le Handler n’a pas a gérer la partie fonctionnel et la classe n’a pas a filtrer l’événement, c’est le role de l’handler. Si Minecraft Forge change son système d’event, tu es sur que seul tes classes technique sont impacté, pas tes classes fonctionnelles.

Bonne idée dans le cas où les blocks sont simple, c’est à dire hérité de la classe Block, néanmoins dans le cas où les block sont hérité d’un block plus particulier, par exemple les escaliers, la classe générique n’a presque aucun intérêt si ce n’est préciser la creativeTab;

En effet, je n’y avais pas pensé. Du coups, je pense qu’il faut supprimer ce point la.
L’avantage de ce système c’était de pouvoir détecter facilement le mod du block avec un instanceof, idem pour les items. A la place, c’est surement mieux de faire une classe Blocks qui possède chaque instance de bloc en static (Comme ce qui est fait avec les block vanilla) et une methode static containsBlock(Block b) qui indique si le block est bien ajouté par le mod.
J’édit le post.

J’ajoute le point sur les enums a la liste

@‘Phenix246’:

Il faut savoir qu’ eclipse comme le compilateur se moquent d’où sont rangées nos classes et leur nom. Le but des conventions est qu’un développeur puisse en comprendre un autre

J’ai laissé traîner un copier/coller foiré, effectivement ça n’a rien a voir avec les montés de version xD

Phenix246

@‘Blackout’:

Sincèrement je n’utilise des proxy principalement que lorsque j’appelle des méthodes coté client, quand c’est coté serveur (commun) j’appelle les méthode directement

Ce qui n’est pas correct sémantiquement. Ta classe principale est un wrapper, elle n’a normalement aucun autre role que d’initialiser ton mod. Les proxies par contre sont les modules de ton mod, le module commun et le module client. A quoi servent les proxy sinon ? Dans ton cas, a quoi sert ton common proxy ? Pourquoi ne pas faire mettre un attribut a null dans ta classe principale que tu instancie uniquement si tu es en client-side dans ton @EventHandler init() ?

Les proxy sont utilisés afin de ne déclarer un élément que d’un coté, ce qui peut paraître bizard avec Forge et Minecraft, est que la partie dit serveur est commune, on a besoin de l’instance du block aussi bien pour avoir sa texture et son rendu (client) , que pour l’update de block (serveur). Ainsi l logique pure voudrais qu’il n’y ait aucun lien entre le proxy client et serveur, or en général le proxy client est hérité du proxy commun ou serveur. Avec cette manière de codé nous pouvons en effet mettre le code dit commun dans le proxy serveur.

Je tiens vu que tu parle de sémantique qu’il est écrit “clientSide” et “serverSide” lorsque tu déclare ton proxy, ce qui montre bien qu’il n’est pas sensé avoir 2 parties aucunement liés.

@‘Blackout’:

Il me parait logique de regrouper dans une même classe tout les Events relatif au fonctionnement par exemple pour l’initialisation et la sauvegarde des playerProps. Néanmoins aller jusqu’à créer des classe avec des méthode appelées lorsque ces events sont déclenchés, je ne trouve pas ça logique, tu appelles des méthodes en cascade ce qui n’est pas considéré comme une bonne manière de développer en général.

Tu trouve par exemple en entreprise des architectures n-tiers ou chaque tiers font un appel délégué a la couche suivante. (Présentation - Metier - DAO - BDD), tu retrouves le même shémas dans la modèle de communication OSI avec ses 8 couches. Un design pattern du nom de business delegate se base la dessus. Donc si c’était considéré comme une mauvaise manière de développer, ça n’existerait pas ^^ L’utilisation d’appel en cascade doit être justifié. Ici, c’est la séparation de la couche technique avec la couche fonctionnelle. Le Handler doit se charger d’effectuer les vérifications sur la side etc et servir de filtre. Par exemple, imaginons une classe qui a besoin d’effectuer une action lorsque le joueur fait un clique droit. La classe va posséder une methode onRightClick(…) et le handler va s’occupé de géré la side et de filtré l’event PlayerInteractEvent pour que la methode onRightClick soit appelé uniquement lorsque le joueur fait un clique droit. Le Handler n’a pas a gérer la partie fonctionnel et la classe n’a pas a filtrer l’événement, c’est le role de l’handler. Si Minecraft Forge change son système d’event, tu es sur que seul tes classes technique sont impacté, pas tes classes fonctionnelles.

Dans un premier temps ne confond pas tout, le modèle OSI ou TCP/IP est un modèle de communication par couche, néanmoins chaque couche à un rôle précis (encoder les données, les envoyer au bon destinataire, etc).

Ensuite sache que Forge dispose déjà d’un système d’évents plutôt complet, il n’est donc pas nécessaire de forcément faire 10 tests afin de se retrouver dans la situation dans laquelle ton code doit fonctionner. Dans le cas où ton code peut directement marcher, a ne sert à rien selon moi de séparer le code en plusieurs partie.

Cependant Dans certain cas afin de clarifié le code ou d’utiliser un event dans un système plus complexe nécessitant plusieurs event pour bien marcher, il peut s’avérer plus pratique de séparer le code en plusieurs éléments.

Je te met en garde pour finir que on ne peut pas appliquer les principes de programmation à tout les autres domaines.

RedRelay

@‘Phenix246’:

Les proxy sont utilisés afin de ne déclarer un élément que d’un coté, ce qui peut paraître bizard avec Forge et Minecraft, est que la partie dit serveur est commune, on a besoin de l’instance du block aussi bien pour avoir sa texture et son rendu (client) , que pour l’update de block (serveur). Ainsi l logique pure voudrais qu’il n’y ait aucun lien entre le proxy client et serveur, or en général le proxy client est hérité du proxy commun ou serveur. Avec cette manière de codé nous pouvons en effet mettre le code dit commun dans le proxy serveur.

Je tiens vu que tu parle de sémantique qu’il est écrit “clientSide” et “serverSide” lorsque tu déclare ton proxy, ce qui montre bien qu’il n’est pas sensé avoir 2 parties aucunement liés.

Je viens de comprendre ton point de vue.
En effet, avec l’architecture de Minecraft il y a 2 écoles :

Ceux qui considèrent le proxy Server Side comme un composant destiné au serveur dédié uniquement et le Client Side destiné au client complet (donc avec serveur local intégré)
Ceux qui considèrent le proxy Server Side comme un composant commun destiné au client et au serveur et le Client Side comme une extension du composant commun pour le client uniquement.
Tout dépend de la terminologie qu’on donne au mot serveur, étant donné qu’il est intégré dans le client.

Je suis plutôt de la seconde école, d’où mon point de vue sur le role des proxy.
Contrairement a toi qui est dans la première école. Effectivement dans ce cas la, tout ce que tu dis prend son sens. Tu as raison.
Je vire ce point de la convention, c’est plus une règle projet qu’une règle générale.

@‘Phenix246’:

Dans un premier temps ne confond pas tout, le modèle OSI ou TCP/IP est un modèle de communication par couche, néanmoins chaque couche à un rôle précis (encoder les données, les envoyer au bon destinataire, etc).

Ensuite sache que Forge dispose déjà d’un système d’évents plutôt complet, il n’est donc pas nécessaire de forcément faire 10 tests afin de se retrouver dans la situation dans laquelle ton code doit fonctionner. Dans le cas où ton code peut directement marcher, a ne sert à rien selon moi de séparer le code en plusieurs partie.

Cependant Dans certain cas afin de clarifié le code ou d’utiliser un event dans un système plus complexe nécessitant plusieurs event pour bien marcher, il peut s’avérer plus pratique de séparer le code en plusieurs éléments.

Je te met en garde pour finir que on ne peut pas appliquer les principes de programmation à tout les autres domaines.

Je ne confond pas tout ^^ Dans le modèle OSI, chaque couche a un role précis, idem dans l’architecture n-tiers. Idem ici.
Je suis conscient que les principes de programmation enterprise ne s’applique pas forcément dans le monde du JV, l’inverse est aussi vrai. Dans l’enterprise, on vise la maintenabilité a long terme plus que les performances.
C’est pour ça que je n’ai pas repris tout les mécanismes d’une architecture n-tier par exemple, car on ferait trop d’instanciation d’objet et donc ça affecterai drastiquement les perfs.
Cependant Minecraft est un jeu qui dure et qui vit sur le long terme, ce n’est pas un Crysis qui une fois compilé ne va plus être modifié sauf avec 5 patchs maxi.
Dans Minecraft, on a cette notion de long terme.

Certes les appels en cascade diminuent les perfs vis a vis de la pile d’exécution, mais est ce que c’est réellement visible ?
N’a-t’on pas plus a gagner qu’a perdre quand on fait le rapport maintenabilité+complexité / performances perdues ?

robin4002

Pour les événements appelé de tant en temps ça devrait aller.
Par contre pour les événement appelé à chaque tick j’éviterai de faire des appels en cascade.

Concernant les proxys, j’ai pris l’habitude d’utiliser ClientProxy pour tout ce qui est client seulement et ServerProxy (que je nomme en plus CommonProxy …) pour le serveur seulement. C’est une mauvaise habitude que j’ai pris car les premiers tutoriels que j’avais suivis était comme ça (et du-coup je répands ma mauvaise habitude dans les tutoriels, bravo Robin x))
Je dis mauvaise habitude car il est incohérent de nommé CommonProxy quelque chose qui ne sert que le pour le serveur.

En fait une manière plus propre de faire serait d’avoir 3 classes. Une classe CommonProxy qui est utilisé pour l’instance dans la classe principale, une classe ClientProxy qui est indiqué dans l’annotation @SidedProxy et est une classe fille de CommonProxy et une classe ServerProxy qui comme ClientProxy est utilisé dans @SidedProxy et est une classe fille de CommonProxy. Chaque fonction du CommonProxy sont override dans ServerProxy et ClientProxy et elles ont en plus un super.nomDeLaFonction(); pour exécuter la fonction dans CommonProxy.
Avec un tel choix les fonctions dans CommonProxy seront toujours exécuté, celle de ClientProxy seulement dans le cas du client et ServerProxy seulement dans le cas du serveur dédiée. Les noms des classes seraient alors beaucoup plus cohérent.
Je pense que je vais utiliser cette façon de faire dans les tutoriels 1.8 et +.

jglrxavpok

Je suis plutôt de la deuxième école dans le sens où je considère le proxy “serveur” comme proxy commun aux deux côtés.

De plus, je conseille de nommer ses noms de méthodes de manière abstraite dans les proxies, de façon à éviter un “registerRenderThings” dans un proxy commun/serveur et plutôt utiliser des noms tels que “preInit”, “init”, etc.

RedRelay

@‘robin4002’:

Pour les événements appelé de tant en temps ça devrait aller.
Par contre pour les événement appelé à chaque tick j’éviterai de faire des appels en cascade.

Concernant les proxys, j’ai pris l’habitude d’utiliser ClientProxy pour tout ce qui est client seulement et ServerProxy (que je nomme en plus CommonProxy …) pour le serveur seulement. C’est une mauvaise habitude que j’ai pris car les premiers tutoriels que j’avais suivis était comme ça (et du-coup je répands ma mauvaise habitude dans les tutoriels, bravo Robin x))
Je dis mauvaise habitude car il est incohérent de nommé CommonProxy quelque chose qui ne sert que le pour le serveur.

En fait une manière plus propre de faire serait d’avoir 3 classes. Une classe CommonProxy qui est utilisé pour l’instance dans la classe principale, une classe ClientProxy qui est indiqué dans l’annotation @SidedProxy et est une classe fille de CommonProxy et une classe ServerProxy qui comme ClientProxy est utilisé dans @SidedProxy et est une classe fille de CommonProxy. Chaque fonction du CommonProxy sont override dans ServerProxy et ClientProxy et elles ont en plus un super.nomDeLaFonction(); pour exécuter la fonction dans CommonProxy.
Avec un tel choix les fonctions dans CommonProxy seront toujours exécuté, celle de ClientProxy seulement dans le cas du client et ServerProxy seulement dans le cas du serveur dédiée. Les noms des classes seraient alors beaucoup plus cohérent.
Je pense que je vais utiliser cette façon de faire dans les tutoriels 1.8 et +.

Ça me semble être la meilleur solution ! Ça résould tout !