CMD #141
Keine Reviewer
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
2 Beteiligte
Fällig am
Kein Fälligkeitsdatum gesetzt.
Abhängigkeiten
Keine Abhängigkeiten gesetzt.
Referenz: SteamWar/SpigotCore#141
Laden…
In neuem Issue referenzieren
Einen Benutzer sperren
Keine Beschreibung angegeben.
Branch "CMD" 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?
CommandSystem-Stuff kann ich nicht beurteilen/reviewen, über den Rest habe ich aber eine deutliche Meinung! :)
@ -36,3 +38,4 @@
private static final Map<UUID, SteamwarUser> byUUID = new HashMap<>();
private static final Map<String, SteamwarUser> byName = new HashMap<>();
private static final Map<Integer, SteamwarUser> byId = new HashMap<>();
private static final Map<String, List<SteamwarUser>> byPrefix = new HashMap<>();
Wie häufig wird byPrefix ausgeführt? Je nachdem (wenn das nur selten (auf ausdrückliche Useraktion/Befehl) hin passiert, sollten wir das nicht cachen.
Zudem: Wird das auch schon mit 1 Buchstaben ausgeführt? Das wäre extrem ungeil, weil wir dann mehr oder weniger auf einmal 1/26 von derzeit 8000+ Usern laden (in etwa, oder wenn . oder % eingegeben werden, alle). Und das halten wir dann im RAM.
Stimme ich zu und siehe unten.
@ -93,6 +97,27 @@ public class SteamwarUser {
return bedrock;
}
public static List<SteamwarUser> getByPrefix(String userName){
Bitte hier unbedingt ausgibige Filterung (., %, zu kurze userNames) einbauen, damit unsere DB nicht so extensiv gequeriet wird.
Falls du diese Funktion für Tab-Completion einbaust: Generell bei Tab-Completion nur die User anbieten, die derzeit auf dem selben Server sind. Der Rest wird deutlich seltener gebraucht und würde nur die Datenbank (und Userverbindung) zuspammen. Wir würden auch dadurch einem User verraten, welche User alles schon mal bei uns waren (und welche nicht).
Wie unten schon gesagt, würde ich ganz gerne alle User die gerade auf dem Bungee sind tabCompleten und nicht nur die die auf dem Server selber sind, dafür gibt es Player.
@ -96,0 +101,4 @@
if (userName.isEmpty()) {
return new ArrayList<>();
}
List<SteamwarUser> users = byPrefix.get(userName.toLowerCase().substring(0, 1));
Warum nur nach dem ersten Buchstaben suchen?
@ -96,0 +113,4 @@
}
return steamwarUsers;
}, userName.toLowerCase() + "%");
byPrefix.put(userName.toLowerCase().substring(0, 1), users);
Wenn das nur der erste Buchstabe ist, ist das dann bei längeren anderen Suchqueries nicht falsch, da zunächst nur mit längerem Suchquery geladen wird?
Ich mache danach noch ein sort darauf selber, deswegen immer nur den ersten. Ich würde gerne nur die die gerade Online sind nehmen, aber wie komme ich an ALLE die auf dem Bungee sind vom Server aus?
Gar nicht, deshalb Limitation auf die, die mit auf dem selben Spigotserver sind (du dürftest auf dem Spigot ja auch nur Befehle haben, welche sich nur auf Leute auf dem selben Spigot beziehen - daher würde das meiner Meinung nach ausreichen)
@ -0,0 +1,122 @@
/*
Die testsrc/.gitkeep kann doch mittlerweile eigentlich weg, oder?
Da hast du recht.
@ -0,0 +19,4 @@
package de.steamwar.command;
public class ExecutionIdentifier extends RuntimeException {
Brauchst du den wirklich, oder hast du andernfalls silent-exceptions die irgendwo gecatcht und verworfen werden? Das wäre dann nämlich eher ein Zeichen davon, dass die Codearchitektur bescheiden ist. Eigentlich sollte man eine korrekte Ausführung daran erkennen, dass der Code ordnungsgemäß terminiert und Werte, welche sich ändern sollten, auf den korrekten Wert geändert wurden.
Naja ich will im test wissen, ob die methode wirklich ausgeführt wird, und das geht am besten indem ich wenn es ausgeführt wird eine Exception werfe und diese überprüfe.
Dann weißt du aber nie, ob der Code, welcher danach noch ausgeführt wird, nicht den Wert wieder zurücksetzen würde/fehler werfen würde.
Das brauche ich mit dem Test nicht prüfen, ich will dort erstmal nur prüfen ob an sich die methode ausgeführt wird, andere Tests werden noch folgen. Aber soweit tut das Framework
Bis auf die Commandsachen (die ich nicht sinnvoll überprüfen kann) ok.
@ -26,6 +26,7 @@ import org.bukkit.entity.Player;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.*;
import java.util.stream.Collectors;
Du und deine Import-Sauberkeit ;).