WIP: CommandFramework #84

YoyoNow · 2020-12-27T18:17:44+01:00

YoyoNow hat

2020-12-27 18:17:44 +01:00

kommentiert

Besitzer

Keine Beschreibung angegeben.

YoyoNow hat

2020-12-27 19:51:51 +01:00

kommentiert

Autor

Besitzer

Benötigt:

Erste Argument int, String, Player..., https://github.com/JorelAli/CommandAPI?
Subargumente

Tasks:

INT, LONG, FLOAT, DOUBLE, STRING, PLAYER (Done)
TabComplete (Done)
Easy SWCommand bundleing (Done)

Benötigt: * Erste Argument int, String, Player..., https://github.com/JorelAli/CommandAPI? * Subargumente Tasks: * INT, LONG, FLOAT, DOUBLE, STRING, PLAYER (Done) * TabComplete (Done) * Easy SWCommand bundleing (Done)

YoyoNow hat

2020-12-27 23:09:41 +01:00

kommentiert

Autor

Besitzer

Als kleines Beispiel der Command API. Sollte eigentlich selbstverständlich sein. Der optional ist true oder false als Wert, wenn der command valid war also die Argument map ohne fehler geparsed werden konnte. Ansonsten ist dieses Optional einfach emtpy().

public static void main(String[] args) {
    SWCommand swCommand = new SWCommand((player, argumentMap) -> {
        System.out.println(argumentMap);
        return false;
    }, ofIgnoreCase("show"), STRING, between(0, 10, 1), DOUBLE);

    Optional<Boolean> booleanOptional = swCommand.execute(null, new String[]{"show", "", "0", "0"});
    Optional<List<String>> optionalStringList = swCommand.tabComplete(new String[]{"show", "Hello", ""});
    System.out.println(booleanOptional);
    System.out.println(optionalStringList);
}

Ausgabe dann:

ArgumentMap{String=show, String=, Integer=0, Double=0.0}
Optional[false]
Optional[[1]]

Als kleines Beispiel der Command API. Sollte eigentlich selbstverständlich sein. Der optional ist true oder false als Wert, wenn der command valid war also die Argument map ohne fehler geparsed werden konnte. Ansonsten ist dieses Optional einfach emtpy(). ```java public static void main(String[] args) { SWCommand swCommand = new SWCommand((player, argumentMap) -> { System.out.println(argumentMap); return false; }, ofIgnoreCase("show"), STRING, between(0, 10, 1), DOUBLE); Optional<Boolean> booleanOptional = swCommand.execute(null, new String[]{"show", "", "0", "0"}); Optional<List<String>> optionalStringList = swCommand.tabComplete(new String[]{"show", "Hello", ""}); System.out.println(booleanOptional); System.out.println(optionalStringList); } ``` Ausgabe dann: ``` ArgumentMap{String=show, String=, Integer=0, Double=0.0} Optional[false] Optional[[1]] ```

YoyoNow hat den Titel von ~~WIP CommandFramework~~ zu WIP: CommandFramework 2020-12-29 13:32:07 +01:00 geändert

Lixfel hat

2020-12-29 21:55:49 +01:00

kommentiert

Besitzer

Erste strukturelle Anmerkungen:

return false; <- Warum sollten wir einen Boolean-Returnwert brauchen? Wenns einen Programmfehler gibt, werfen wir den auch, und ansonsten ist alles ok.

Die ArgumentMap finde ich nicht ganz so elegant, weil man da ja dann irgendwie alle Argumente dann wieder rausfischen muss. Am elegantesten fände ich eine Lösung a'la:

public TraceCommand(){
  register(trace_show); // Wird so nicht funktionieren, aber so ähnlich stelle ich mir das vor
}

//ShowMode und ShowOption hier als Enum möglicher Werte
public void trace_show(@Nullable ShowMode showMode, @Nullable ShowOption options...){
   //Blabla Code
}

Offene Frage dabei wäre da natürlich: Wie wird ein Integer/Double vom Wertebereich eingegrenzt?

Anonsten würde ich mir bei deinem Vorschlag doch gerne mal ein Beispiel ansehen, in der auf die ArumentMap zugegriffen wird, und zwar auf einzelne Argumente. Frage: Was macht bei between(,,1) die 1? Und müsste beim tabComplete nicht beim TabCompleteaufruf alle Werte von 1-10 zurückgegeben werden?

Erste strukturelle Anmerkungen: return false; <- Warum sollten wir einen Boolean-Returnwert brauchen? Wenns einen Programmfehler gibt, werfen wir den auch, und ansonsten ist alles ok. Die ArgumentMap finde ich nicht ganz so elegant, weil man da ja dann irgendwie alle Argumente dann wieder rausfischen muss. Am elegantesten fände ich eine Lösung a'la: ``` public TraceCommand(){ register(trace_show); // Wird so nicht funktionieren, aber so ähnlich stelle ich mir das vor } //ShowMode und ShowOption hier als Enum möglicher Werte public void trace_show(@Nullable ShowMode showMode, @Nullable ShowOption options...){ //Blabla Code } ``` Offene Frage dabei wäre da natürlich: Wie wird ein Integer/Double vom Wertebereich eingegrenzt? Anonsten würde ich mir bei deinem Vorschlag doch gerne mal ein Beispiel ansehen, in der auf die ArumentMap zugegriffen wird, und zwar auf einzelne Argumente. Frage: Was macht bei between(,,1) die 1? Und müsste beim tabComplete nicht beim TabCompleteaufruf alle Werte von 1-10 zurückgegeben werden?

Lixfel hat

2020-12-29 22:02:18 +01:00

kommentiert

Besitzer

Nach erster oberflächlicher Betrachtung des Codes: Der ArgumentBuilder schaut mir unbenötigt/überflüssig aus.

Argument und ArgumentType schauen mir irgendwie doppelt gemoppelt zu sein. Was unterscheidet die beiden so fundamental bzw. braucht es wirklich beides? Zudem: Wenn ich einen Player möchte, ist das dann wirklich ein String-Argument? Ich möchte ja schließlich am Ende einen Spieler in meinem Command haben und keinen String.

Nach erster oberflächlicher Betrachtung des Codes: Der ArgumentBuilder schaut mir unbenötigt/überflüssig aus. Argument und ArgumentType schauen mir irgendwie doppelt gemoppelt zu sein. Was unterscheidet die beiden so fundamental bzw. braucht es wirklich beides? Zudem: Wenn ich einen Player möchte, ist das dann wirklich ein String-Argument? Ich möchte ja schließlich am Ende einen Spieler in meinem Command haben und keinen String.

YoyoNow hat

2020-12-29 22:03:02 +01:00

kommentiert

Autor

Besitzer

Dieser boolean sollte eigentlich für den CommandExecutor sein. Ich kann aber auch einfach boolean nehmen für gab es einen valid command und false dann wenn nicht?

Lixfel hat

2020-12-29 22:04:03 +01:00

kommentiert

Besitzer

Dieser boolean sollte eigentlich für den CommandExecutor sein. Ich kann aber auch einfach boolean nehmen für gab es einen valid command und false dann wenn nicht?

Wann ist ein Command bitte invalid? (bzw. ein CommandExecutor)?!?

> Dieser boolean sollte eigentlich für den CommandExecutor sein. Ich kann aber auch einfach boolean nehmen für gab es einen valid command und false dann wenn nicht? Wann ist ein Command bitte invalid? (bzw. ein CommandExecutor)?!?

Lixfel hat

2020-12-29 22:07:45 +01:00

kommentiert

Besitzer

Ok, ich habe gerade mal wieder eine "simple" Idee, die wir wenn wsl. auch schrittweise einführen könnten und schon mal einen Schritt in Richtung eines Frameworks wäre:

interface Argument<T>{
        List<String> tabComplete(CommandSender sender, String arg);
        T parse(CommandSender sender, String arg) throws InvalidArgumentException;
    }

Ein abstrahiertes Interface, dessen Implementierungen dann auch schon so jetzt von existierenden Commands genutzt werden könnten.

Ok, ich habe gerade mal wieder eine "simple" Idee, die wir wenn wsl. auch schrittweise einführen könnten und schon mal einen Schritt in Richtung eines Frameworks wäre: ``` interface Argument<T>{ List<String> tabComplete(CommandSender sender, String arg); T parse(CommandSender sender, String arg) throws InvalidArgumentException; } ``` Ein abstrahiertes Interface, dessen Implementierungen dann auch schon so jetzt von existierenden Commands genutzt werden könnten.

YoyoNow hat

2020-12-29 22:08:47 +01:00

kommentiert

Autor

Besitzer

ArgumentType gibt den "primitive" Type an. Und einen mapper, womit man zwischen den String und dem wirklichen Type konvertieren kann. Ein Argument baut dann auf dieses ArgumentType auf und gibt einen Mapper an womit man das dann zum Beispiel zu einem Player objekt "casten" kann. Man könnte diese beiden Sachen mergen. Jedoch weiß ich nicht wie groß der Code dabei wird.

Lixfel hat

2020-12-29 22:10:12 +01:00

kommentiert

Besitzer

YoyoNow, für mich ist auch ein Player ein primitiver Typ. Was unterscheidet einen Integer und einen Player?

YoyoNow hat

2020-12-29 22:14:22 +01:00

kommentiert

Autor

Besitzer

Ich finde dein Interface oben ganz interessant. Ich gucke mal ob ich damit das ganze etwas eleganter machen kann. Ich würde dann ganz gerne trotzdem vorgefehrtigte Argument's angbieten für Material Player usw.

YoyoNow hat

2020-12-29 22:15:32 +01:00

kommentiert

Autor

Besitzer

Warum sollte man für das Parsen bitte einen CommandSender benötigen?

YoyoNow hat

2020-12-29 22:17:33 +01:00

kommentiert

Autor

Besitzer

Man könnte sowas ähnliches machen. Jedoch wäre das was du unten gesagt hast eher viel Reflections als eine ordentliche schöne Lösung.

Erste strukturelle Anmerkungen:

return false; <- Warum sollten wir einen Boolean-Returnwert brauchen? Wenns einen Programmfehler gibt, werfen wir den auch, und ansonsten ist alles ok.

Die ArgumentMap finde ich nicht ganz so elegant, weil man da ja dann irgendwie alle Argumente dann wieder rausfischen muss. Am elegantesten fände ich eine Lösung a'la:
public TraceCommand(){
  register(trace_show); // Wird so nicht funktionieren, aber so ähnlich stelle ich mir das vor
}

//ShowMode und ShowOption hier als Enum möglicher Werte
public void trace_show(@Nullable ShowMode showMode, @Nullable ShowOption options...){
   //Blabla Code
}
Offene Frage dabei wäre da natürlich: Wie wird ein Integer/Double vom Wertebereich eingegrenzt?

Anonsten würde ich mir bei deinem Vorschlag doch gerne mal ein Beispiel ansehen, in der auf die ArumentMap zugegriffen wird, und zwar auf einzelne Argumente. Frage: Was macht bei between(,,1) die 1? Und müsste beim tabComplete nicht beim TabCompleteaufruf alle Werte von 1-10 zurückgegeben werden?

Man könnte sowas ähnliches machen. Jedoch wäre das was du unten gesagt hast eher viel Reflections als eine ordentliche schöne Lösung. > Erste strukturelle Anmerkungen: > > return false; <- Warum sollten wir einen Boolean-Returnwert brauchen? Wenns einen Programmfehler gibt, werfen wir den auch, und ansonsten ist alles ok. > > Die ArgumentMap finde ich nicht ganz so elegant, weil man da ja dann irgendwie alle Argumente dann wieder rausfischen muss. Am elegantesten fände ich eine Lösung a'la: > ``` > public TraceCommand(){ > register(trace_show); // Wird so nicht funktionieren, aber so ähnlich stelle ich mir das vor > } > > //ShowMode und ShowOption hier als Enum möglicher Werte > public void trace_show(@Nullable ShowMode showMode, @Nullable ShowOption options...){ > //Blabla Code > } > ``` > Offene Frage dabei wäre da natürlich: Wie wird ein Integer/Double vom Wertebereich eingegrenzt? > > Anonsten würde ich mir bei deinem Vorschlag doch gerne mal ein Beispiel ansehen, in der auf die ArumentMap zugegriffen wird, und zwar auf einzelne Argumente. Frage: Was macht bei between(,,1) die 1? Und müsste beim tabComplete nicht beim TabCompleteaufruf alle Werte von 1-10 zurückgegeben werden?

Lixfel hat

2020-12-29 22:20:07 +01:00

kommentiert

Besitzer

Warum sollten Reflections eine unsaubere Lösung sein? (Gut, evtl. ist meine Lösung da etwas stark Pythonesk.

Ich würde ebenfalls vorgefertigte Argumente anbieten, wozu man einen CommandSender braucht? Kleiner Beispiel (eher Bungee-Bezogen, aber ansonsten auch nicht anders). Man könnte übrigens auch über Vererbung deines "SWCommands" nachdenken, um das Commandframework zu nutzen.

protected static final Argument<ProxiedPlayer> PLAYER_ARG = new Argument<ProxiedPlayer>() {
        @Override
        public List<String> tabComplete(CommandSender sender, String arg) {
            List<String> players = new ArrayList<>();
            if(arg == null)
                ProxyServer.getInstance().getPlayers().forEach(player -> players.add(player.getName()));
            else
                ProxyServer.getInstance().getPlayers().forEach(player -> {
                    if(player.getName().startsWith(arg))
                        players.add(player.getName());
                });
            return players;
        }

        @Override
        public ProxiedPlayer parse(CommandSender sender, String arg) {
            ProxiedPlayer player = ProxyServer.getInstance().getPlayer(arg);
            if(player == null){
                sender.sendMessage("PREFIX: §cUnbekannter Spieler " + arg);
                throw new IllegalArgumentException();
            }
            return player;
        }
    };

Warum sollten Reflections eine unsaubere Lösung sein? (Gut, evtl. ist meine Lösung da etwas stark Pythonesk. Ich würde ebenfalls vorgefertigte Argumente anbieten, wozu man einen CommandSender braucht? Kleiner Beispiel (eher Bungee-Bezogen, aber ansonsten auch nicht anders). Man könnte übrigens auch über Vererbung deines "SWCommands" nachdenken, um das Commandframework zu nutzen. ``` protected static final Argument<ProxiedPlayer> PLAYER_ARG = new Argument<ProxiedPlayer>() { @Override public List<String> tabComplete(CommandSender sender, String arg) { List<String> players = new ArrayList<>(); if(arg == null) ProxyServer.getInstance().getPlayers().forEach(player -> players.add(player.getName())); else ProxyServer.getInstance().getPlayers().forEach(player -> { if(player.getName().startsWith(arg)) players.add(player.getName()); }); return players; } @Override public ProxiedPlayer parse(CommandSender sender, String arg) { ProxiedPlayer player = ProxyServer.getInstance().getPlayer(arg); if(player == null){ sender.sendMessage("PREFIX: §cUnbekannter Spieler " + arg); throw new IllegalArgumentException(); } return player; } }; ```

YoyoNow hat

2020-12-29 22:28:01 +01:00

kommentiert

Autor

Besitzer

Lixfel. Ich habe mal in einem neuen packet dein Ansatz in 2 Klassen zusammengeschmissen. Man müsste wohl noch was Struktur außenrum machen. unteranderem zum bündeln von parametern. Und vllt noch ein wenig mehr was das parsen angeht. Ich habe erstmal ein IntArgument noch gebaut, was einfach ein Integer kann und ein DoubleArgument. Da Könnte man noch einbauen, das diese Ranges darin passieren. Ich würde des weiteren nochmal überlegen ob man im parse() call nicht gleich auch sowas wie 'predicate' checks machen kann: Soll heißen das checken ob der Parameter korrekt ist und dort dann auch eine InvalidArgumentException schmeißen kann. Wie würdest du da weiter machen. Ich glaube Wäre noch sowas ganz wichtig, womit man überprüfen kann ob ein Input überhaupt auf dieses Argument passt. Dafür würde ich deinen Code im interface um eine weitere methode erweitern:

boolean checkTypeValidity(String arg);

Auch würde ich da vllt den check ob das Argument korrekt ist in einen eigenen Interface call auslagern, sodass man parsing und checking eindeutig trennt. Hierfür würde ich:

default boolean checkArgumentConstraints(T t) {
    return true;
}

noch in das Interface einbauen.

Lixfel. Ich habe mal in einem neuen packet dein Ansatz in 2 Klassen zusammengeschmissen. Man müsste wohl noch was Struktur außenrum machen. unteranderem zum bündeln von parametern. Und vllt noch ein wenig mehr was das parsen angeht. Ich habe erstmal ein IntArgument noch gebaut, was einfach ein Integer kann und ein DoubleArgument. Da Könnte man noch einbauen, das diese Ranges darin passieren. Ich würde des weiteren nochmal überlegen ob man im parse() call nicht gleich auch sowas wie 'predicate' checks machen kann: Soll heißen das checken ob der Parameter korrekt ist und dort dann auch eine InvalidArgumentException schmeißen kann. Wie würdest du da weiter machen. Ich glaube Wäre noch sowas ganz wichtig, womit man überprüfen kann ob ein Input überhaupt auf dieses Argument passt. Dafür würde ich deinen Code im interface um eine weitere methode erweitern: ```java boolean checkTypeValidity(String arg); ``` Auch würde ich da vllt den check ob das Argument korrekt ist in einen eigenen Interface call auslagern, sodass man parsing und checking eindeutig trennt. Hierfür würde ich: ```java default boolean checkArgumentConstraints(T t) { return true; } ``` noch in das Interface einbauen.

YoyoNow hat

2020-12-29 22:28:54 +01:00

kommentiert

Autor

Besitzer

Wie meinst du das genau mit der vererbung des SWCommand? Ich würde den umbauen, damit das neue Argument System das kann?

YoyoNow hat

2020-12-29 22:31:06 +01:00

kommentiert

Autor

Besitzer

Also mit Reflections wäre dein Ansatz oben mit der Methode möglich. Jedoch wäre die intern einfach groß, bedeutet viele Zeilen. Und nicht gut Wartbar, außer wenn man Reflections in und auswendig kennt. Ich glaube ich würde sowas zwar schaffen jedoch einfach nicht so, dass es irgendwie gut nutzbar bzw erweiterbar ist.

YoyoNow hat

2020-12-29 22:32:25 +01:00

kommentiert

Autor

Besitzer

Auch dürfte bei deinem Beispiel arg niemals 'null' sein es wäre empty bzw ein leerer string aber eigentlich nicht null.

YoyoNow hat

2020-12-29 22:34:57 +01:00

kommentiert

Autor

Besitzer

Ich glaube ein solchen Code:

ProxyServer.getInstance().getPlayers().forEach(player -> {
                    if(player.getName().startsWith(arg))
                        players.add(player.getName());
                });

Sollte man irgendwo als utility methode nehmen, da dieses überall in diesem System dann verwendet werden würde. Ich glaube, dass hier es einfacher wäre das in SWCommand zu bünden, dass dieser die Auswahl macht, wäre glaube eleganter als so?

Ich glaube ein solchen Code: ``` ProxyServer.getInstance().getPlayers().forEach(player -> { if(player.getName().startsWith(arg)) players.add(player.getName()); }); ``` Sollte man irgendwo als utility methode nehmen, da dieses überall in diesem System dann verwendet werden würde. Ich glaube, dass hier es einfacher wäre das in SWCommand zu bünden, dass dieser die Auswahl macht, wäre glaube eleganter als so?

YoyoNow hat

2020-12-29 22:38:24 +01:00

kommentiert

Autor

Besitzer

Also ich würde persönlich lieber die parse methode einfach nur für das parsen nehmen, also es gibt keinen CommandSender, womit man dann nur den String den man bekommt umwandeln soll in den passenden Type der erwartet wird. Um dann über die checkArgumentConstraints dann den rest zu machen. Wo du dann auch den CommandSender bekommen würdest. Ich glaube das wäre eine Wesehntlich elegantere Lösung, da ich parse als namen echt dann etwas irreführend finde.

Lixfel hat

2020-12-30 07:53:28 +01:00

kommentiert

Besitzer

Würde es wie schon gesagt ohne TabCompleteFilter machen, und es erscheint mir unnötig, extra Klassen für die einzelnen Typen a'la IntArgument zu machen, schließlich brauchen wir da jeweils nur ein Objekt von, und keine Klasse.

Wobei beim Int könnte es sogar Sinn machen, dann kann man für die verschiedenen Constraints dann immer in den Parse-Methoden super.parse() erstmal aufrufen und dann die Range checken.

Würde es wie schon gesagt ohne TabCompleteFilter machen, und es erscheint mir unnötig, extra Klassen für die einzelnen Typen a'la IntArgument zu machen, schließlich brauchen wir da jeweils nur ein Objekt von, und keine Klasse. Wobei beim Int könnte es sogar Sinn machen, dann kann man für die verschiedenen Constraints dann immer in den Parse-Methoden super.parse() erstmal aufrufen und dann die Range checken.

YoyoNow hat 1 Commit 2021-03-12 10:57:31 +01:00 hinzugefügt

Fix Argument 050fcafc71

YoyoNow hat

2021-03-12 11:25:11 +01:00

kommentiert

Autor

Besitzer

![image](/devlabs/attachments/d0616087-a293-4729-89c1-68e491c0d6b7)

image.png

304 KiB

YoyoNow hat diesen Pull-Request

2021-04-09 16:04:47 +02:00