ModCommand #445
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
4 Beteiligte
Fällig am
Kein Fälligkeitsdatum gesetzt.
Abhängigkeiten
Keine Abhängigkeiten gesetzt.
Referenz: SteamWar/BungeeCore#445
Laden…
In neuem Issue referenzieren
Einen Benutzer sperren
Keine Beschreibung angegeben.
Branch "ModCommand" 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?
Ermöglicht das einstufen von mods ingame, sowie das finden von uneingestuften mods.
ModCommandzu WIP: ModCommandStatt /classifymod next evtl. direkt /classifymod? Dazu evtl. die Möglichkeit, die Klassifizierungsoptionen direkt im Chat anzuklicken, oder besser eine GUI (wegen Autoscrolling). Dann hast du auch nicht die Namensambiguität.
@ -163,6 +163,8 @@ public class BungeeCore extends Plugin {
new CalendarCommand();
new CalendarListener();
new ModCommand();
Wird das CommandFramework nicht anders initialisiert?
nein. und es funktioniert so
@ -0,0 +13,4 @@
public class ModCommand extends SWCommand {
private static final Statement set = new Statement("UPDATE Mods Set ModType = ? WHERE ModName = ?");
Der Modname ist nicht eindeutig (Modloader spielt auch noch eine Rolle)
@ -0,0 +16,4 @@
private static final Statement set = new Statement("UPDATE Mods Set ModType = ? WHERE ModName = ?");
private static final Statement findFirst = new Statement("SELECT * FROM Mods WHERE ModType = 0 LIMIT 1");
private static final Statement get = new Statement("SELECT * FROM Mods WHERE ModName = ?");
Bitte wenn direkt korrekt mit allem drum und dran im SQL-Modul als Mod-Klasse machen. Allgemein gerade schwieriger Zeitpunkt, weil das auch irgendwann demnächst in den CommonCore kommt.
Wie meinst du das mi als Mod-Klasse? Würde ich interpretieren vom test her das ich die ganze klasse ins sql modul schiebe.
@ -0,0 +19,4 @@
private static final Statement get = new Statement("SELECT * FROM Mods WHERE ModName = ?");
public ModCommand() {
super("mod", "bungeecore.teamchat", "mods");
Bitte nur /classifymod, und die Berechtigung sollte eher an bungeecore.softreload gebunden werden. Wobei ich denke, dass das doch hier nicht korrekt ist mit dem CommandFramework, das arbeitet doch gar nicht mit diesen Berechtigungen?
werde ich bald testen
@ -0,0 +27,4 @@
SteamwarUser user = SteamwarUser.get(p.getUniqueId());
UserGroup group = user.getUserGroup();
if(!group.isAdminGroup()) {
Auch hier.
@ -0,0 +34,4 @@
boolean modExists = get.select(ResultSet::next,modName);
System.out.println(modExists);
DebugCode
@ -0,0 +46,4 @@
Message.send("MOD_CHANGED_TYPE",p,modName,newModType);
}
@Register(value = {"next"},description = "MOD_COMMAND_NEXT_USAGE")
Die Description braucht es nicht.
@ -0,0 +51,4 @@
SteamwarUser user = SteamwarUser.get(p.getUniqueId());
UserGroup group = user.getUserGroup();
if(!group.isAdminGroup()) {
Ich glaube, dafür gibt es schon eine Restriktionsmöglichkeit im CommandSystem, bitte die direkt dafür nutzen.
@ -0,0 +68,4 @@
return;
}
Message.send("MOD_FOUND_NEXT_MOD",p,foundMod);
Bitte noch die Information hinzufügen, welcher Modloader.
@ -32,4 +31,3 @@
import net.md_5.bungee.api.chat.ClickEvent;
import net.md_5.bungee.api.connection.ProxiedPlayer;
import java.net.InetSocketAddress;
Unrelated Changes bitte entfernen (das gibt dir sonst Mergeconflicts mit deinem Whois-Branch)
Nur mal so ne Anmerkung:
Mein Plan war so ein Frontend für die Mods-Tabelle auch in das EventTool ein zu bauen (Chaoscaot/steamwar_multitool#4)
Das halte ich für den falschen Ort, die Ingame-Lösung finde ich viel komfortabler.
Eine nicht Ingame-Lösung ist nicht an ein TUI oder ein Kisten-GUI gebunden, was die Darstellung um einiges verbessert, imho.
warum nicht beides
Weil ich kaum glaube, das Chaoscaot eine deutlich bessere grafische Repräsentation als phpMyAdmin geschaffen hat.
die commands bleiben erstmal, gui werde ich aber parralel dazu einarbeiten.
WIP: ModCommandzu ModCommand@ -0,0 +23,4 @@
private static final Statement get = new Statement("SELECT * FROM Mods WHERE ModName = ? AND Platform = ?");
private static final Statement getAll = new Statement("SELECT * FROM Mods ORDER BY ModName DESC LIMIT ?, ?");
SQL-Statements gehören in das SQL-Package, da sollte es eine Mod-Klasse geben, da gehören die rein
@ -0,0 +31,4 @@
@Register()
public void genericCommand(ProxiedPlayer p) {
SWStreamInv<ModEntry> swStreamInv = new SWStreamInv<>(p,Message.parse("MOD_COMMAND_GUI_TITLE",p), (click, element) -> {
Das Inventar im aktuellen Status ist etwas Fraglich, es zeigt einem alle Mods an (ca. 1700, 38 Seiten).
Es sollte nur die nicht-Klassifizierten anzeigen oder alle mit ein Paar Filter funktionen eg. Name, Klassifizierung
@ -0,0 +45,4 @@
int modPlatform = element.platform.get();
swInventory.addItem(2,new SWItem("GRAY_CONCRETE","Unclassified"), (click1 -> set.update(0,modName,modPlatform)));
swInventory.addItem(3,new SWItem("GREEN_CONCRETE", "Allowed"), (click1 -> set.update(1,modName,modPlatform)));
Nach dem Komma (überall) sollte auch eine Leerzeichen hin, aber wenn dann nicht noch mischen.
@ -0,0 +47,4 @@
swInventory.addItem(2,new SWItem("GRAY_CONCRETE","Unclassified"), (click1 -> set.update(0,modName,modPlatform)));
swInventory.addItem(3,new SWItem("GREEN_CONCRETE", "Allowed"), (click1 -> set.update(1,modName,modPlatform)));
swInventory.addItem(4,new SWItem("YELLOW_CONCRETE", "Pending"),(click1 -> set.update(2,modName,modPlatform)));
swInventory.addItem(5,new SWItem("RED_CONCRETE","Forbidden"),(click1 -> set.update(3,modName,modPlatform)));
Funktionieren die Dinger auch pre-Flattinging (pre 1.13), vllt. Farbstoffe?
@ -0,0 +48,4 @@
swInventory.addItem(3,new SWItem("GREEN_CONCRETE", "Allowed"), (click1 -> set.update(1,modName,modPlatform)));
swInventory.addItem(4,new SWItem("YELLOW_CONCRETE", "Pending"),(click1 -> set.update(2,modName,modPlatform)));
swInventory.addItem(5,new SWItem("RED_CONCRETE","Forbidden"),(click1 -> set.update(3,modName,modPlatform)));
swInventory.addItem(6,new SWItem("PURPLE_CONCRETE", "YT_only"),(click1 -> set.update(4,modName,modPlatform)));
Namen an besten auch in die .properties Datei und keine Unterstriche als Leerzeichen
@ -0,0 +119,4 @@
return;
}
Message.send("MOD_FOUND_NEXT_MOD",p,foundMod.getKey(), Mod.Platform.getByValue(foundMod.getValue()));
Hier sollten noch Klickbare Nachrichten zum einstufen hinzukommen
@ -0,0 +122,4 @@
Message.send("MOD_FOUND_NEXT_MOD",p,foundMod.getKey(), Mod.Platform.getByValue(foundMod.getValue()));
}
private class ModEntry {
Kann auch durch Mod klasse ersetzt werden
@ -91,6 +102,7 @@ public class Mod {
ModType(int value){
this.value = value;
}
@Getter
In Zeile 71 gibts so etwas ähnliches, wird auch oben genutzt anstatt
getValue
Irgendwie habe ich bei dem Branch das Gefühl, man habe einfach mal sämtliche Ideen von Objektorientierung und Kapselung aus dem Fenster geworfen und damit alles komplexer gemacht, als es hätte sein müssen.
@ -0,0 +1,191 @@
package de.steamwar.bungeecore.commands;
Fehlender License-Header
@ -0,0 +26,4 @@
public ModCommand() {
super("mod", "bungeecore.softreload", "mods");
}
private static FilterType filtertype = FilterType.ALL;
Das das dann der Filter für alle Nutzer zeitgleich angepasst/gesetzt wird, halte ich für ziemlich problematisch.
@ -0,0 +28,4 @@
}
private static FilterType filtertype = FilterType.ALL;
@Register()
Klammer hier unnötig.
@ -0,0 +35,4 @@
private void openGui(ProxiedPlayer p) {
SWStreamInv<ModEntry> swStreamInv = new SWStreamInv<>(p,Message.parse("MOD_COMMAND_GUI_TITLE",p), (click, element) -> {
SWInventory swInventory = new SWInventory(p,9,Message.parse("MOD_COMMAND_GUI",p));
Könnten wir diese Lambda-Funktion (mit nur dem Spieler und Mod als Argument) bitte der Lesbarkeit wegen in eine separate Methode auslagern?
@ -0,0 +40,4 @@
String modName = element.modName;
int modPlatform = element.platform.get();
swInventory.addItem(2, new SWItem(Message.parse("MOD_UNCLASSIFIED",p),8), (click1 -> Mod.set.update(0, modName, modPlatform)));
Nach dem Klassifizieren würde ich direkt wieder die Hauptgui öffnen (es ist unwahrscheinlich, dass man den gleichen Mod hintereinander mehrfach klassifizieren möchte)
@ -0,0 +51,4 @@
openGui(p);
});
swInventory.open();
In dem Fall wird hier anders als in Zeile 50 die GUI nicht vorher geschlossen. Musst du testen/untersuchen, ob das gemacht werden sollte und beide Fälle nach Untersuchungsergebnis vereinheitlichen.
Ich tue das in den anderen fällen weil es sich komisch verhalten hat. Hier tut es ohne probleme
@ -0,0 +136,4 @@
Message.send("MOD_COMMAND_INFO",p,modName,platform.name(),Mod.ModType.valueOf(getModType(modName,platform.get())).name());
}
private int getModType(String modName,int modPlatform) {
Wenn du das Mod-Object hast, weißt du schon, welcher Typ das ganze ist...
@ -0,0 +164,4 @@
Message.send("MOD_FOUND_NEXT_MOD",p,"MOD_OPEN_GUI",new ClickEvent(ClickEvent.Action.RUN_COMMAND,""),foundMod.getKey(),Mod.Platform.getByValue(foundMod.getValue()));
}
private enum FilterType {
Wie wäre es, statt ALL null zu verwenden und dann einfach stattdessen das reguläre Mod.ModType-Enum?
@ -0,0 +179,4 @@
}
}
private class ModEntry {
Warum hier eine separate WrapperClasse und nicht einfach Mod verwenden?
@ -32,4 +31,3 @@
import net.md_5.bungee.api.chat.ClickEvent;
import net.md_5.bungee.api.connection.ProxiedPlayer;
import java.net.InetSocketAddress;
Die Änderungen dieser Datei bitte aus diesem Branch entfernen.
@ -22,3 +24,3 @@
public class Mod {
private static final Statement get = new Statement("SELECT * FROM Mods WHERE ModName = ? AND Platform = ?");
public static final Statement get = new Statement("SELECT * FROM Mods WHERE ModName = ? AND Platform = ?");
Nein, das bleibt definitiv private.
@ -27,0 +31,4 @@
public static final Statement getAll = new Statement("SELECT * FROM Mods ORDER BY ModName DESC LIMIT ?, ?");
public static final Statement getAllFiltered = new Statement("SELECT * FROM Mods WHERE ModType = ? ORDER BY ModName DESC LIMIT ?, ?");
Auch hier alle private. Das SQL-Zeugs bleibt in .sql. Du reichst dann wenn eine List<Mod> nach draußen, alles außerhalb hat nichts mit SQL am Hut.
@ -72,2 +81,4 @@
return value;
}
public static Platform getByValue(int val) {
Die Methode braucht es nicht, schließlich sind Mod-Objects schon entsprechend geparst.
@ -81,3 +99,3 @@
YOUTUBER_ONLY(4);
static ModType valueOf(int value){
public static ModType valueOf(int value){
lass es private/Package-private.
@ -91,6 +109,7 @@ public class Mod {
ModType(int value){
this.value = value;
}
@Getter
Dann nicht mehr benötigt.
@ -659,0 +665,4 @@
MOD_FOUND_NEXT_MOD=§7Next unclassified mod is {0}!
MOD_COMMAND_NOT_FOUND_IN_DATABASE=§7The Mod {0} on platform {1} was§c not §7found in the database!
MOD_COMMAND_INFO=§7The mod {0} on platform {1} is of the type {2}.
MOD_COMMAND_GUI_TITLE=§7Unclassified Mods
GUI-Titel sind die einzigen Fälle, wo kein Color-Coding stattfinden sollte.
@ -659,0 +674,4 @@
MOD_FORBIDDEN=Forbidden
MOD_YT=YT Only
MOD_ALL=All
MOD_ITEM_BACK=Back
Color-Coding bei diesem Eintrag und den Vorhergehenden fehlend.
Und bitte auch eine deutsche Übersetzung!
jap, die sind noch wip
Was mir jetzt erstmal nur so aufgefallen ist (habe noch nicht alles durchgeschaut)
@ -32,4 +31,3 @@
import net.md_5.bungee.api.chat.ClickEvent;
import net.md_5.bungee.api.connection.ProxiedPlayer;
import java.net.InetSocketAddress;
Chrmm. Diesen Changediff bitte aufräumen. (Du hast die Datei anderweitig nicht verändert).
@ -135,6 +136,7 @@ public class ConnectionListener extends BasicListener {
public void onDisconnect(PlayerDisconnectEvent e){
ChallengeCommand.remove(e.getPlayer());
MsgCommand.remove(e.getPlayer());
ModCommand.playerFilterType.remove(e.getPlayer());
Das gibt einen MergeConflict mit deinem anderen Branch (nur schonmal vorneweg die Warnung)
@ -47,6 +62,53 @@ public class Mod {
return new Mod(modName, platform, ModType.UNKLASSIFIED);
}
public static Mod get(String modName, int platform) {
Die Methode dürfte nach den anderen Änderungen nicht mehr nötig sein.
@ -50,0 +66,4 @@
return get(modName,Mod.Platform.valueOf(platform));
}
public static ModType getModType(String modName,Mod.Platform platform) {
Mach daraus doch bitte eine normale Member-Methode. (also getModType(){return modType;})
@ -50,0 +70,4 @@
return get(modName,platform).modType;
}
public static void setMod(Mod.ModType newModType,String modName,Mod.Platform platform) {
Bitte umbenennen in setModType und in Member-Funktion umwandeln (wie getModType)!
@ -50,0 +74,4 @@
set.update(newModType.value ,modName,platform.value);
}
public static boolean doesModExist(String modName,Mod.Platform modPlatform) {
Schöner wäre (weil ansonsten doppelter Datenbankzugriff) wenn du die derzeitige get()-Methode umbennenen würdest in z.B. getOrCreate() und den eigentlichen get() Anteil der Methode in eine neue get()-Methode auslagerst, welche einfach null zurückliefert, wenn der Mod nicht existiert. (Weniger Codeduplication, weniger DB-Calls)
@ -50,0 +82,4 @@
return Mod.getAll.select(rs -> {
List<Mod> f = new ArrayList<>();
while(rs.next()){
Mod entry = new Mod(rs.getString("ModName"), Mod.Platform.valueOf(rs.getInt("Platform")),Mod.ModType.valueOf(rs.getInt("ModType")));
Es wäre evtl. geschickt, einen neuen Konstruktor, welcher ein ResultSet nimmt und SQLExceptions wirft zu machen.
@ -50,0 +83,4 @@
List<Mod> f = new ArrayList<>();
while(rs.next()){
Mod entry = new Mod(rs.getString("ModName"), Mod.Platform.valueOf(rs.getInt("Platform")),Mod.ModType.valueOf(rs.getInt("ModType")));
f.add(entry);
du kannst das new inlinen (also f.add(new Mod...))
@ -50,0 +89,4 @@
},page * elementsPerPage, elementsPerPage);
}
public static List<Mod> getAllModsFiltered(int page,int elementsPerPage, Mod.ModType filter) {
Statt filtered würde ich eher getAllModsOfType verwenden... (auch SQL-Statement so umbenennen)
damit würde ich aber permanent einen filter applien. wenn ich dies aber nicht möchte, brauch ich das statement ohne filter
Was ja auch der Methodenname "ofType" impliziert. Damit ändert sich ja nichts am Methodeninhalt, nur statt filtered - ofType im Namen.
@ -50,0 +100,4 @@
},filter.value, page * elementsPerPage, elementsPerPage);
}
public static Pair<String,Integer> findFirstMod() {
Gib hier doch einfach einen Mod zurück. Spart die obskuren apache-Imports und macht dir doch auch sonst das Leben einfacher, oder?
@ -68,2 +130,4 @@
this.value = value;
}
static Platform valueOf(int value){
Sollte mit den anderen Änderungen nicht mehr nötig sein.
durch static kontext noch nötig
Merge conflicts bitte beheben.
Merge conflicts bitte nicht vergessen. Deine SQL-Klasse ist immer noch absoluter murks.
@ -20,2 +20,4 @@
package de.steamwar.bungeecore.sql;
import lombok.Getter;
import org.apache.commons.lang3.tuple.ImmutablePair;
Unused imports bitte verwerfen.
@ -48,2 +63,4 @@
}
public static Mod get(String modName, int platform) {
return get(modName,Mod.Platform.valueOf(platform));
Hier wird ggf immer noch der Mod erstellt! Bitte benenne die andere Methode getOrCreate um, rufe diese hier nicht auf und nimm hier keinen int als Argument! Nutze dann stattdessen in getOrCreate diese get-Methode (keine Code duplication).
@ -50,0 +70,4 @@
return get(modName,platform).modType;
}
public void setModType(Mod.ModType newModType,String modName,Mod.Platform platform) {
Bitte modName und platform aus den Argumenten entfernen und von dieser Mod-Instanz verwenden.
@ -50,0 +92,4 @@
findFirst.select(rs -> {
String name = rs.getString("ModName");
int platform = rs.getInt("Platform");
return get(name,Platform.valueOf(platform));
Du hast hier doch bereits schon auch den ModType, warum erstellst du nicht direkt ein Mod-Objekt? So machst du stattdessen nochmal einen Datenbankaufruf!
@ -50,0 +94,4 @@
int platform = rs.getInt("Platform");
return get(name,Platform.valueOf(platform));
});
return null;
Wie wäre es, statt immer null zu returnen, dein Ergebnis von findFirst zu returnen?
Bitte noch einmal ausgiebig testen @zOnlyKroks ! Da waren immer noch offensichtliche Bugs drinnen, und ich weiß nicht, ob ich alle erwischt habe.
@ -0,0 +1,147 @@
/*
This file is a part of the SteamWar software.
Copyright (C) 2020 SteamWar.de-Serverteam
Du kannst dein Template auch mal Updaten :D