CommandFramework3 #94
Label
Kein Label
Bug
Codeverbesserung
Einsteiger Freundlich
Idee
In Arbeit
Neues Feature
Prio A
Security Breach
Überprüfung notwendig
Verbesserung
Zu Beobachten
Kein Meilenstein
Niemand zuständig
3 Beteiligte
Fällig am
Kein Fälligkeitsdatum gesetzt.
Abhängigkeiten
Keine Abhängigkeiten gesetzt.
Referenz: SteamWar/SpigotCore#94
Laden…
In neuem Issue referenzieren
Einen Benutzer sperren
Keine Beschreibung angegeben.
Branch "CommandFramework3" löschen
Das Löschen eines Branches ist permanent. Obwohl der Branch für eine kurze Zeit weiter existieren könnte, kann diese Aktion in den meisten Fällen NICHT rückgängig gemacht werden. Fortfahren?
Command System
Todos
@RegisterHelp([value])
?) ->@Register(help = true)
Beispiel
Als kleines Beispiel, wie man das neue System verwenden würde:
Scheint mir sehr angenehm zu bedienen sein, finde ich grundlegend gut.
Ansonsten scheinen mir das Hilfsmeldungssystem wie der Command funktioniert noch nicht so gut ausgebaut zu sein.
@ -0,0 +28,4 @@
import java.util.List;
import java.util.stream.Collectors;
public class TestCommand extends SWCommand {
Ich fände es besser, wenn es ein Implements sein würde, dann kann die Klasse auch noch anderes als ein Befehl sein.
Im Moment geht dies nicht, wegen dem Constructor, und den Sachen, welche dort drin passieren. Dementsprechend ist dies so im Moment nicht möglich. Wie würdest du das umsetzten?
Ich würde ungern mit defaults und so arbeiten, deswegen habe ich es im Moment so gelöst.
@ -0,0 +32,4 @@
public TestCommand() {
// Register this command as 'test'
super("test");
Prinzipiell gut, aber insbesondere im Kampfsystem (nach refactoring) tausche ich die Implementierungen von Befehlen während der Serverlaufzeit mehrfach aus, je nachdem, welche Kampfphase gerade ist. Wäre toll, wenn das mit dem System gehen würde.
Kannst du mir hier zu ein Beispiel geben, was genau du brauchen wirst?
Reicht dir die Implementierung von an und ausschalten von Befehlen mit dem enabled boolean? Oder brauchst du mehr als das?
Wenn ja sollten wir mal am Wochenende darüber reden.
@ -0,0 +50,4 @@
// Add Custom Mapper when this command is registered, all Mapper of you class will be
// created first and than the Commands. Do not use this mapper outside your class, as
// it can only create failures. Use on your own risk. Definition order should be considered.
Liest sich arg hacky, ich fände es gut, wenn Mapperobjekte nur einmal erstellt werden und ggf. von mehreren Befehlen genutzt werden könnten. Evtl. eine Annotation @Mapper mit einem TypeMapper als Argument statt einem String? Dann haben wir auch nicht mehr die dreckige string-lösung.
Ich wollte die Mapper explizit von dem Parameter weghaben, damit diese explizit auch global erzeugt werden können. Das was du da liest ist eine abkürzung, womit du Mapper, welche du nur hier in diesem Befehl brauchst hier definierst und verwendest. Ab dem Moment wird es für alle Befehle, welche danach erzeugt werden auch existieren. Der String ist explizit, damit man ein Argument 'Material' haben kann, welches jedoch einen anderen Mapper haben kann. Der name ist dann auch so gut zu wählen wie nötig oder möglich.
Die Mapper an sich werden auch nur einmal erzeugt und dann in eine Map intern gepackt. Außerdem kann man in Annotationen keine Interfaces als fields verwenden:
Deswegen geht deine Idee leider nicht, hatte ich auch schon als Idee.
Lixfel das Help System bin ich auch noch am überarbeiten. Da habe ich noch keine direkte Idee wie ich es lösen soll. Aber vllt kommt mir ja noch eine.
Das Help System könnte nun so deinen Vorstellungen nun mehr entsprechen.
@ -0,0 +52,4 @@
// Another Command, subCommands can be implemented with the Register Annotation,
// you can use custom Mappers by putting a Mapper Annotation on a Parameter
@Register({"two"})
public void testTwo(Player player, int i, @Mapper("solidMaterial") Material material) {
Das das ganze mit Strings funktioniert, gefällt mir immer noch nicht (und dir wsl. auch nicht). Da anscheinend keine Interfaces, sondern nur konkrete Instanzen einer Klasse verwendet werden können: Mach doch eine Klasse TypeMapper, die als Parameter eine Funktion String -> Objekt nimmt. Dann ist das eine klare Klasse. Ggf. ist das dann erstmal ein Object (wenn Templating nicht geht) ansonsten spricht aber meines Wissens nach nix dagegen, oder?
Ich verstehe dein Vorschlag nicht ganz. Du willst, dass ich in der Annotation eine Object angebe, welches dann eine Function<String, Object> beinhaltet. Wie soll das das Mapper zeug lösen. Kannst du versuchen das nochmal etwas genauer zu beschreiben?
Das hier spricht dagegen (Allowed Types in Annotation):
Multidimensional arrays are forbidden. Arrays of type Class are forbidden.
Hier warum letzteres verboten ist:
'Constant Expressions' ist das Stichwort
@ -0,0 +31,4 @@
public abstract class SWCommand {
private boolean enabled = true;
Das ist nicht so das, was ich fürs FightSystem gemeint habe. Ich möchte eigentlich nicht, dass sich der Command merkt, ob er jetzt enabled oder disabled ist, sondern den Command einfach Registrieren und aber auch wieder Entregistrieren können. Dann kann ich nämlich auch bestimmen, wass der Command macht, wenn er "disabled" ist, oder gar komplexere State-Machines umsetzen.
Ok ich gucke, dass ich das eingebaut bekomme, an sich muss ich ja nur unregister können. Weril registerieren tust du ja mit einer Instanz erzeugen,
Dies sollte nun möglich sein.
@ -0,0 +32,4 @@
public abstract class SWCommand {
private boolean enabled = true;
private final Set<SubCommand> commandSet = new HashSet<>();
Da du glaube sowieso keinen Befehl doppelt einfügst und dann häufig drüberiterierst, wäre glaube ich eine ArrayList angebrachter.
Ich glaube eher eine LinkedList, weil ich nur drüber iteriere oder?
Nein, der Vorteil einer Linkedlist ist eher nur bei häufigem Entfernen aus der Mitte gegeben. Die ArrayList ist auch beim Iterieren schneller, weil da ja einfach nur der index um eins erhöht werden muss (bessere Speicherpositionierung)
@ -0,0 +54,4 @@
};
static {
addMapper(boolean.class, Boolean.class, createMapper(Boolean::parseBoolean, s -> Arrays.asList("true", "false")));
Wird wirklich irgendwo als Parameter "Boolean" benötigt? Oder "Float"? Oder...
Ich finde Boolean und Float ist benötigt, da man diese Mapper von außen nicht so schön erzeugen kann und wir sollten etwas Zukunftsicher an sich sache gehen, was benötigt wird.
@ -0,0 +97,4 @@
} catch (IllegalArgumentException | IllegalAccessException e) {
throw new SecurityException(e.getMessage(), e);
} catch (InvocationTargetException | RuntimeException e) {
Bukkit.getLogger().log(Level.INFO, e.getMessage(), e);
Ich denke mal, das throw new SecurityException aus der Zeile drüber wäre auch angebracht. Auf jeden Fall für "RuntimeException".
@ -0,0 +105,4 @@
List<String> tabComplete(CommandSender commandSender, String[] args) {
if (!varArgs && args.length < arguments.length - 1) {
return Collections.emptyList();
Entweder du behandelst im SWCommand nicht den Fall null, oder du returnst immer eine leere Liste. Bitte nicht beides zeitgleich.
Denn fall null behandle ich eigentlich wegen unsauberen TabCompletern, die eigengeschrieben sind.
Dann nutze doch auch einfach immer null.
WIP: CommandFramework3zu CommandFramework3Exzessiv getestet
Der merge kam aufgrund eines Missverständnisses zwischen Yoyo und mir, sry D;
Kaputt gehen kann aber davon nix