SteamWar/BungeeCore
Archiviert
13
2

ModCommand #445

Zusammengeführt
Lixfel hat 21 Commits von ModCommand nach master 2023-02-14 09:13:46 +01:00 zusammengeführt
3 geänderte Dateien mit 106 neuen und 34 gelöschten Zeilen
Nur Änderungen aus Commit 69aa98e82e werden angezeigt - Alle Commits anzeigen

Datei anzeigen

@ -1,5 +1,6 @@
package de.steamwar.bungeecore.commands;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Fehlender License-Header

Fehlender License-Header
import de.steamwar.bungeecore.BungeeCore;
import de.steamwar.bungeecore.Message;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Du kannst dein Template auch mal Updaten :D

Du kannst dein Template auch mal Updaten :D
import de.steamwar.bungeecore.inventory.SWInventory;
import de.steamwar.bungeecore.inventory.SWItem;
@ -7,6 +8,9 @@ import de.steamwar.bungeecore.inventory.SWListInv;
import de.steamwar.bungeecore.inventory.SWStreamInv;
import de.steamwar.bungeecore.sql.*;
import de.steamwar.command.SWCommand;
import lombok.Getter;
import net.md_5.bungee.BungeeCord;
import net.md_5.bungee.api.chat.ClickEvent;
import net.md_5.bungee.api.connection.ProxiedPlayer;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Der Modname ist nicht eindeutig (Modloader spielt auch noch eine Rolle)

Der Modname ist nicht eindeutig (Modloader spielt auch noch eine Rolle)
@ -14,43 +18,72 @@ import org.apache.commons.lang3.tuple.Pair;
import java.sql.ResultSet;
import java.util.ArrayList;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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.

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.
Veraltet
Review

Wie meinst du das mi als Mod-Klasse? Würde ich interpretieren vom test her das ich die ganze klasse ins sql modul schiebe.

Wie meinst du das mi als Mod-Klasse? Würde ich interpretieren vom test her das ich die ganze klasse ins sql modul schiebe.
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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?

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?
Veraltet
Review

werde ich bald testen

werde ich bald testen
public class ModCommand extends SWCommand {
private static final Statement set = new Statement("UPDATE Mods Set ModType = ? WHERE ModName = ? AND Platform = ?");
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 = ? AND Platform = ?");
private static final Statement getAll = new Statement("SELECT * FROM Mods ORDER BY ModName DESC LIMIT ?, ?");
public ModCommand() {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

SQL-Statements gehören in das SQL-Package, da sollte es eine Mod-Klasse geben, da gehören die rein

SQL-Statements gehören in das SQL-Package, da sollte es eine Mod-Klasse geben, da gehören die rein
super("mod", "bungeecore.softreload", "mods");
}
private static FilterType filtertype = FilterType.ALL;
zOnlyKroks markierte diese Unterhaltung als gelöst
Review

Das das dann der Filter für alle Nutzer zeitgleich angepasst/gesetzt wird, halte ich für ziemlich problematisch.

Das das dann der Filter für alle Nutzer zeitgleich angepasst/gesetzt wird, halte ich für ziemlich problematisch.
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Auch hier.

Auch hier.
@Register()
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Klammer hier unnötig.

Klammer hier unnötig.
public void genericCommand(ProxiedPlayer p) {
openGui(p);
}
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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

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
private void openGui(ProxiedPlayer p) {
SWStreamInv<ModEntry> swStreamInv = new SWStreamInv<>(p,Message.parse("MOD_COMMAND_GUI_TITLE",p), (click, element) -> {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

DebugCode

DebugCode
openGradingWindow(p,element);
},page -> getMods(page,45).stream().map(mod -> new SWListInv.SWListEntry<>(getModItem(mod),mod)).collect(Collectors.toList()));
SWInventory swInventory = new SWInventory(p,9,Message.parse("MOD_COMMAND_GUI",p));
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Könnten wir diese Lambda-Funktion (mit nur dem Spieler und Mod als Argument) bitte der Lesbarkeit wegen in eine separate Methode auslagern?

Könnten wir diese Lambda-Funktion (mit nur dem Spieler und Mod als Argument) bitte der Lesbarkeit wegen in eine separate Methode auslagern?
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)));
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Nach dem Klassifizieren würde ich direkt wieder die Hauptgui öffnen (es ist unwahrscheinlich, dass man den gleichen Mod hintereinander mehrfach klassifizieren möchte)

Nach dem Klassifizieren würde ich direkt wieder die Hauptgui öffnen (es ist unwahrscheinlich, dass man den gleichen Mod hintereinander mehrfach klassifizieren möchte)
swInventory.addItem(3, new SWItem(Message.parse("MOD_ALLOWED",p), 2), (click1 -> Mod.set.update(1, modName, modPlatform)));
swInventory.addItem(4, new SWItem(Message.parse("MOD_PENDING",p), 11), (click1 -> Mod.set.update(2, modName, modPlatform)));
swInventory.addItem(5, new SWItem(Message.parse("MOD_FORBIDDEN",p),1), (click1 -> Mod.set.update(3, modName, modPlatform)));
swInventory.addItem(6, new SWItem(Message.parse("MOD_YT",p), 13), (click1 -> Mod.set.update(4, modName, modPlatform)));
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Nach dem Komma (überall) sollte auch eine Leerzeichen hin, aber wenn dann nicht noch mischen.

Nach dem Komma (überall) sollte auch eine Leerzeichen hin, aber wenn dann nicht noch mischen.
swInventory.addItem(8,new SWItem("ARROW",Message.parse("MOD_ITEM_BACK",p)), click1 -> {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Die Description braucht es nicht.

Die Description braucht es nicht.
swInventory.close();
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Funktionieren die Dinger auch pre-Flattinging (pre 1.13), vllt. Farbstoffe?

Funktionieren die Dinger auch pre-Flattinging (pre 1.13), vllt. Farbstoffe?
openGui(p);
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Namen an besten auch in die .properties Datei und keine Unterstriche als Leerzeichen

Namen an besten auch in die .properties Datei und keine Unterstriche als Leerzeichen
});
swInventory.open();
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Ich glaube, dafür gibt es schon eine Restriktionsmöglichkeit im CommandSystem, bitte die direkt dafür nutzen.

Ich glaube, dafür gibt es schon eine Restriktionsmöglichkeit im CommandSystem, bitte die direkt dafür nutzen.
Veraltet
Review

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.

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.
Veraltet
Review

Ich tue das in den anderen fällen weil es sich komisch verhalten hat. Hier tut es ohne probleme

Ich tue das in den anderen fällen weil es sich komisch verhalten hat. Hier tut es ohne probleme
},page -> {
if(filtertype == FilterType.ALL) {
return getAllMods(page,45).stream().map(mod -> new SWListInv.SWListEntry<>(getModItem(mod),mod)).collect(Collectors.toList());
}else {
return getAllModsFiltered(page,45,filtertype.value).stream().map(mod -> new SWListInv.SWListEntry<>(getModItem(mod),mod)).collect(Collectors.toList());
}
});
swStreamInv.addItem(52,new SWItem("NAME_TAG","Filter"), click -> {
swStreamInv.close();
openFilterGui(p);
});
swStreamInv.open();
}
private void openGradingWindow(ProxiedPlayer p,ModEntry element) {
SWInventory swInventory = new SWInventory(p,9,Message.parse("MOD_COMMAND_GUI",p));
private void openFilterGui(ProxiedPlayer p) {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Bitte noch die Information hinzufügen, welcher Modloader.

Bitte noch die Information hinzufügen, welcher Modloader.
SWInventory inv = new SWInventory(p, 9, "Filter");
String modName = element.modName;
int modPlatform = element.platform.get();
inv.addItem(1, new SWItem(Message.parse("MOD_UNCLASSIFIED",p),8), click -> filtertype = FilterType.UNCLASSIFIED);
inv.addItem(2, new SWItem(Message.parse("MOD_ALLOWED",p),2), click -> filtertype = FilterType.ALLOWED);
inv.addItem(3, new SWItem(Message.parse("MOD_PENDING",p), 11),click -> filtertype = FilterType.PENDING);
inv.addItem(4, new SWItem(Message.parse("MOD_FORBIDDEN",p),1), click -> filtertype = FilterType.FORBIDDEN);
inv.addItem(5, new SWItem(Message.parse("MOD_YT",p),13), click -> filtertype = FilterType.YT);
inv.addItem(6, new SWItem(Message.parse("MOD_ALL",p),15), click -> filtertype = FilterType.ALL);
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)));
swInventory.addItem(6,new SWItem("PURPLE_CONCRETE", "YT_only"),(click1 -> set.update(4,modName,modPlatform)));
inv.addItem(8, new SWItem("ARROW", Message.parse("MOD_ITEM_BACK",p)), click -> {
inv.close();
openGui(p);
});
swInventory.open();
inv.open();
}
private SWItem getModItem(ModEntry modEntry) {
@ -61,51 +94,60 @@ public class ModCommand extends SWCommand {
return item;
}
public List<ModEntry> getMods(int page,int elementsPerPage) {
return getAll.select(rs -> {
public List<ModEntry> getAllMods(int page,int elementsPerPage) {
return Mod.getAll.select(rs -> {
List<ModEntry> f = new ArrayList<>();
while(rs.next()){
ModEntry entry = new ModEntry(rs.getString("ModName"), Mod.Platform.getByValue(rs.getInt("Platform")));
System.out.println(entry.modName);
f.add(entry);
}
return f;
}, page * elementsPerPage, elementsPerPage);
}
public List<ModEntry> getAllModsFiltered(int page,int elementsPerPage, int filter) {
return Mod.getAllFiltered.select(rs -> {
List<ModEntry> f = new ArrayList<>();
while(rs.next()){
ModEntry entry = new ModEntry(rs.getString("ModName"), Mod.Platform.getByValue(rs.getInt("Platform")));
f.add(entry);
}
return f;
},filter, page * elementsPerPage, elementsPerPage);
}
@Register(value = {"set"},description = "MOD_COMMAND_SET_USAGE")
public void set(ProxiedPlayer p,String modName,Mod.Platform platform,Mod.ModType newModType) {
int modPlatform = platform.get();
boolean modExists = get.select(ResultSet::next,modName,modPlatform);
boolean modExists = Mod.get.select(ResultSet::next,modName,modPlatform);
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Hier sollten noch Klickbare Nachrichten zum einstufen hinzukommen

Hier sollten noch Klickbare Nachrichten zum einstufen hinzukommen
if(!modExists) {
Message.send("MOD_COMMAND_NOT_FOUND_IN_DATABASE",p,modName);
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Kann auch durch Mod klasse ersetzt werden

Kann auch durch Mod klasse ersetzt werden
return;
}
set.update(newModType.getValue(),modName,modPlatform);
Mod.set.update(newModType.getValue(),modName,modPlatform);
Message.send("MOD_CHANGED_TYPE",p,modName,newModType.name(),newModType);
}
@Register(value = {"get"},description = "MOD_COMMAND_GET_USAGE")
public void get(ProxiedPlayer p,String modName,Mod.Platform platform) {
Message.send("MOD_COMMAND_INFO",p,modName,platform.name(),Mod.Platform.getByValue(getModType(modName,platform)));
Message.send("MOD_COMMAND_INFO",p,modName,platform.name(),Mod.ModType.valueOf(getModType(modName,platform.get())).name());
}
private int getModType(String modName,Mod.Platform modPlatform) {
return get.select(rs -> {
private int getModType(String modName,int modPlatform) {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Wenn du das Mod-Object hast, weißt du schon, welcher Typ das ganze ist...

Wenn du das Mod-Object hast, weißt du schon, welcher Typ das ganze ist...
return Mod.get.select(rs -> {
if(rs.next()) {
return rs.getInt("ModType");
} else {
return 0;
}
return 0;
},modName,modPlatform);
}
@Register(value = {"next"})
public void next(ProxiedPlayer p) {
Pair<String ,Integer> foundMod = findFirst.select(rs -> {
Pair<String ,Integer> foundMod = Mod.findFirst.select(rs -> {
if(rs.next()) {
String name = rs.getString("ModName");
int platform = rs.getInt("Platform");
@ -119,7 +161,22 @@ public class ModCommand extends SWCommand {
return;
}
Message.send("MOD_FOUND_NEXT_MOD",p,foundMod.getKey(), Mod.Platform.getByValue(foundMod.getValue()));
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 {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Wie wäre es, statt ALL null zu verwenden und dann einfach stattdessen das reguläre Mod.ModType-Enum?

Wie wäre es, statt ALL null zu verwenden und dann einfach stattdessen das reguläre Mod.ModType-Enum?
ALL(-1),
UNCLASSIFIED(0),
ALLOWED(1),
PENDING(2),
FORBIDDEN(3),
YT(4);
final int value;
FilterType(int value) {
this.value = value;
}
}
private class ModEntry {
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Warum hier eine separate WrapperClasse und nicht einfach Mod verwenden?

Warum hier eine separate WrapperClasse und nicht einfach Mod verwenden?

Datei anzeigen

@ -23,9 +23,16 @@ import lombok.Getter;
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Unused imports bitte verwerfen.

Unused imports bitte verwerfen.
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 = ?");
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Nein, das bleibt definitiv private.

Nein, das bleibt definitiv private.
private static final Statement insert = new Statement("INSERT INTO Mods (ModName, Platform) VALUES (?, ?)");
public static final Statement set = new Statement("UPDATE Mods Set ModType = ? WHERE ModName = ? AND Platform = ?");
public static final Statement findFirst = new Statement("SELECT * FROM Mods WHERE ModType = 0 LIMIT 1");
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 ?, ?");
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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.

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.
private final String modName;
private final Platform platform;
private final ModType modType;
@ -91,7 +98,7 @@ public class Mod {
RED(3),
YOUTUBER_ONLY(4);
static ModType valueOf(int value){
public static ModType valueOf(int value){
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

lass es private/Package-private.

lass es private/Package-private.
for(ModType mt : values()){
if(value == mt.value)
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Gib hier doch einfach einen Mod zurück. Spart die obskuren apache-Imports und macht dir doch auch sonst das Leben einfacher, oder?

Gib hier doch einfach einen Mod zurück. Spart die obskuren apache-Imports und macht dir doch auch sonst das Leben einfacher, oder?
return mt;

Datei anzeigen

@ -667,3 +667,11 @@ MOD_COMMAND_NOT_FOUND_IN_DATABASE=§7The Mod {0} on platform {1} was§c not §7f
MOD_COMMAND_INFO=§7The mod {0} on platform {1} is of the type {2}.
MOD_COMMAND_GUI_TITLE=§7Unclassified Mods
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

GUI-Titel sind die einzigen Fälle, wo kein Color-Coding stattfinden sollte.

GUI-Titel sind die einzigen Fälle, wo kein Color-Coding stattfinden sollte.
MOD_COMMAND_GUI=§7Mod Type Changer
MOD_OPEN_GUI=§7Open Gui
MOD_UNCLASSIFIED=Unclassified
MOD_ALLOWED=Allowed
MOD_PENDING=Pending
MOD_FORBIDDEN=Forbidden
MOD_YT=YT Only
MOD_ALL=All
MOD_ITEM_BACK=Back
zOnlyKroks markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Color-Coding bei diesem Eintrag und den Vorhergehenden fehlend.

Und bitte auch eine deutsche Übersetzung!

Color-Coding bei diesem Eintrag und den Vorhergehenden fehlend. Und bitte auch eine deutsche Übersetzung!
Veraltet
Review

jap, die sind noch wip

jap, die sind noch wip