Ajout du wrapper OMCPlayer#1343
Conversation
|
Plop ! des avis ? sur ce que je doit ajouter / ameliorer, etc ? |
…retrieval methods
| return city; | ||
| } | ||
|
|
||
| public class Message { |
There was a problem hiding this comment.
Pour devoir rallonger la ligne en faisant des omcplayer.message().send au lieu de faire OMCPlayer.sendMessagz ?
There was a problem hiding this comment.
parcequ'il y a deja les sendMessage de Player
There was a problem hiding this comment.
Mhh c'est vrai mais tu peux faire des méthodes différentes genre OMCPlayer.sendMessage(Component, Prefix, MessageType, bool)
There was a problem hiding this comment.
dans ce cas la pourquoi faire un OMCPlayer.sendMessage
autant utiliser le MessageManager quoi
There was a problem hiding this comment.
Tu penses vraiment que des gens vont faire send(Component) et qu'ils vont savoir que ça envoie avec un prefix OPEN_MC ? Fin je trouve c'est ce complexifier la vie pour rien.
Quand tu utilises cette méthode les gens ne savent pas forcément de quoi il s'agit et de quoi il est utilisé avant de regarder le code de la méthode
There was a problem hiding this comment.
En vrai stv ping nirbose et toutouchien pour leur demander leur avis car le mien est différent des autres je suppose
There was a problem hiding this comment.
Tu penses vraiment que des gens vont faire send(Component) et qu'ils vont savoir que ça envoie avec un prefix OPEN_MC ? Fin je trouve c'est ce complexifier la vie pour rien. Quand tu utilises cette méthode les gens ne savent pas forcément de quoi il s'agit et de quoi il est utilisé avant de regarder le code de la méthode
bah c'est pour ca que j'ai mis dans une sous class message, et que c'est une methode custom qui vient de OMCPlayer et pas de Player, donc bon...
apres je peux ajouter la JavaDoc hein
There was a problem hiding this comment.
On va attendre d'autres review ptet
There was a problem hiding this comment.
Je veux voir les avis des autres contributeur
|
|
||
| public class City { | ||
| @Nullable | ||
| public fr.openmc.core.features.city.City getCity() { |
There was a problem hiding this comment.
Mhh y'a pas pourtant plusieures classes City 🙃
There was a problem hiding this comment.
Ben je crois que une IA a mis le package entier de la classe fr.openmc.core.features.city.models.City
There was a problem hiding this comment.
CityManager.getCity return fr.openmc.core.features.city.City
There was a problem hiding this comment.
Pourquoi avoir mis le package entier pour détecter la classe City ??
There was a problem hiding this comment.
parceque dans ma class OMCPlayer j'ai deja une class City
There was a problem hiding this comment.
J'aurais pus la rename c'est vrai, mais j'me suis dis ca que maintenant
|
Je vais redodo, je chercherais des choses à mettre, (je regarderai le code pour avoir des idées, j'avais pensé à certaines utiles comme PlayerUtils.hasEnough ou .removeItems ect |
2 participants
Petit résumé de la PR:
Étape nécessaire afin que la PR soit fini (si PR en draft)
[FEAT] Creation d'un OMCPlayer #1147
Decrivez vos changements