SteamWar/SpigotCore
Archiviert
13
0

Add SchematicSelector #132

Zusammengeführt
Lixfel hat 16 Commits von schematic_selector nach master 2021-12-02 08:38:43 +01:00 zusammengeführt
Besitzer

Signed-off-by: Chaoscaot chaoscaot444@gmail.com

Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-21 10:59:55 +01:00 hinzugefügt
Add SchematicSelector
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
22df0035c4
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
YoyoNow hat 1 Commit 2021-11-21 15:17:57 +01:00 hinzugefügt
Merge branch 'master' into schematic_selector
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
7a78fa9d63
Chaoscaot hat 1 Commit 2021-11-22 21:11:04 +01:00 hinzugefügt
Fix Import for Lixfel
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
83d7a4c988
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Lixfel hat 2021-11-23 10:33:57 +01:00 Änderungen angefragt
@ -0,0 +34,4 @@
import java.util.List;
import java.util.function.Consumer;
public class SchematicSelector {
Besitzer

M U L T I L I N G

M U L T I L I N G
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +79,4 @@
openList(null, publicMode == PublicMode.PUBLIC_ONLY);
}
private void openList(SchematicNode parent, boolean publics) {
Besitzer

Könntest du evtl die Megamethoden in kleinere Methoden aufteilen, damit man einen besseren Überblick hat? Und ggf. schauen, ob man nicht noch etwas vereinfachen kann.

Könntest du evtl die Megamethoden in kleinere Methoden aufteilen, damit man einen besseren Überblick hat? Und ggf. schauen, ob man nicht noch etwas vereinfachen kann.
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +210,4 @@
inv.open();
}
private void openFilter(boolean publics) {
Besitzer

Ich würde evtl. nur einen Filter/eine Suche anbieten, welche dafür sowohl nach passenden Besitzern, Namen, Typen und Items zeitgleich durchsucht. Das dürfte für 99% aller Useranfragen völlig gleichwertig sein und den Usern stets einen Klick ersparen.

Ich würde evtl. nur einen Filter/eine Suche anbieten, welche dafür sowohl nach passenden Besitzern, Namen, Typen und Items zeitgleich durchsucht. Das dürfte für 99% aller Useranfragen völlig gleichwertig sein und den Usern stets einen Klick ersparen.
Lixfel markierte diese Unterhaltung als gelöst
@ -0,0 +329,4 @@
return node.getParentNode();
}
public static SelectorTarget selectSchematic() {
Besitzer

Wenn diese Funktionen public sind, sollte auch SelectorTarget public sein

Wenn diese Funktionen public sind, sollte auch SelectorTarget public sein
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +362,4 @@
}
@AllArgsConstructor
private enum Target {
Besitzer

Klasse in der Klasse der Klasse? :/

Klasse in der Klasse der Klasse? :/
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +364,4 @@
@AllArgsConstructor
private enum Target {
SCHEMATIC("Schematic", false),
DIRECTORY("Ordner", true),
Besitzer

Multiling.

Multiling.
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +366,4 @@
SCHEMATIC("Schematic", false),
DIRECTORY("Ordner", true),
SCHEMATIC_NODE("Schematic/Ordner", true),
SCHEMATIC_TYPE("Schematic", false);
Besitzer

Dopplung von SCHEMATIC?

Dopplung von SCHEMATIC?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +24,4 @@
import de.steamwar.sql.SchematicNode;
import org.bukkit.entity.Player;
public abstract class SchematicSelectorInjectableAdapter implements SchematicSelectorInjectable {
Besitzer

Oh je. Jetzt wird das langsam wirklich BigCorporationJavaClassFactory.

Oh je. Jetzt wird das langsam wirklich BigCorporationJavaClassFactory.
Besitzer

Ok, statt das so zu machen: Mach lieber im SchematicSelectorInjectable jede Funktion mit default void ... Default-Implementierungen, dann brauchst du keine separate abstract Class dafür...

Ok, statt das so zu machen: Mach lieber im SchematicSelectorInjectable jede Funktion mit default void ... Default-Implementierungen, dann brauchst du keine separate abstract Class dafür...
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +33,4 @@
public class UtilGui {
public static void openMaterialSelector(Player player, Consumer<Material> callback) {
openMaterialSelector(player, "Material auswählen", callback);
Besitzer

Multiling?

Multiling?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +41,4 @@
for(Material material : Material.values()){
if(material.name().startsWith(Material.LEGACY_PREFIX))
continue;
SWItem item = new SWItem(material, "§7" + material.name());
Besitzer

Multling? (Es müsste theoretisch möglich sein, dem Minecraft-Client das Material so zu übergeben, dass der Client das selbst übersetzt)

Multling? (Es müsste theoretisch möglich sein, dem Minecraft-Client das Material so zu übergeben, dass der Client das selbst übersetzt)
Chaoscaot markierte diese Unterhaltung als gelöst
Chaoscaot hat 1 Commit 2021-11-23 19:01:32 +01:00 hinzugefügt
Doing Lixfel Stuff
Einige Prüfungen sind fehlgeschlagen
SteamWarCI Build failed
bf796ebce3
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-23 19:02:28 +01:00 hinzugefügt
Public
Einige Prüfungen sind fehlgeschlagen
SteamWarCI Build failed
74a4dba9f3
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-23 19:04:18 +01:00 hinzugefügt
Enum in a class in a class
Einige Prüfungen sind fehlgeschlagen
SteamWarCI Build failed
256e76f9bb
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-23 19:05:18 +01:00 hinzugefügt
Fix Build
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
fd420778c1
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat ein Review von Lixfel 2021-11-25 21:10:10 +01:00 angefragt
Chaoscaot hat 1 Commit 2021-11-26 12:57:32 +01:00 hinzugefügt
Merge branch 'master' into schematic_selector
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
6ecd8a5f0e
Lixfel hat 2021-11-27 05:38:40 +01:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Du machst viel zu viele Verhaltensunterscheidungen mit boolean-Flags. Ich empfehle dir mal diesen Talk anzusehen: https://www.youtube.com/watch?v=29MAL8pJImQ

Du machst viel zu viele Verhaltensunterscheidungen mit boolean-Flags. Ich empfehle dir mal diesen Talk anzusehen: https://www.youtube.com/watch?v=29MAL8pJImQ
@ -0,0 +27,4 @@
SCHEM_SELECTOR_NEW_DIR=§7Neuer Ordner
SCHEM_SELECTOR_FILTER=§7Filter
SCHEM_SELECTOR_CREATE_DIR_TITLE=Ordner Erstellen
Besitzer

erstellen

erstellen
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +32,4 @@
SCHEM_SELECTOR_FILTER_TITLE=Filter
SCHEM_SELECTOR_FILTER_ENTER_NAME=Name eingeben
SCHEM_SELECTOR_FILTER_NAME=§7Nach Namen suchen...
SCHEM_SELECTOR_FILTER_NAME_SEARCH=§7Suchwort: §e{0}
Besitzer

Typischer wäre Suchbegriff

Typischer wäre Suchbegriff
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +37,4 @@
SCHEM_SELECTOR_FILTER_OWNER=§7Nach Besitzer suchen...
SCHEM_SELECTOR_FILTER_OWNER_SEARCH=§7Besitzer: §e{0}
SCHEM_SELECTOR_FILTER_SEL_TYPE=Typ wählen...
SCHEM_SELECTOR_FILTER_TYPE=§7Nach Typ Filtern...
Besitzer

filtern

filtern
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +39,4 @@
SCHEM_SELECTOR_FILTER_SEL_TYPE=Typ wählen...
SCHEM_SELECTOR_FILTER_TYPE=§7Nach Typ Filtern...
SCHEM_SELECTOR_FILTER_TYPE_SEARCH=§7Typ: §e{0}
SCHEM_SELECTOR_FILTER_MAT=§7Nach Item Filtern...
Besitzer

filtern

filtern
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +48,4 @@
SCHEM_SELECTOR_DIRECTORY=Ordner
SCHEM_SELECTOR_SCHEMATIC_NODE=Schematic/Ordner
MATERIAL_SELECTOR_TITLE=Material auswählen
Besitzer

Mit Material hat der User eher weniger am Hut. Item?

Mit Material hat der User eher weniger am Hut. Item?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +38,4 @@
public class SchematicSelector {
public static final String DEFAULT_TITLE = "SCHEM_SELECTOR_TITLE";
Besitzer

Warum public?

Warum public?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +46,4 @@
private final SteamwarUser user;
@Getter
private final Consumer<SchematicNode> callback;
@Getter
Besitzer

Getter macht keinen Sinn, keine Klasse außerhalb kann ohne Reflections auf Parameter von SelectorTarget zugreifen.

Getter macht keinen Sinn, keine Klasse außerhalb kann ohne Reflections auf Parameter von SelectorTarget zugreifen.
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +60,4 @@
private boolean singleDirOpen;
@Setter
@Getter
private Function<Player, String> title;
Besitzer

Hast du für solche Sachen nicht eigentlich den SchematicSelectorInjectable?

Hast du für solche Sachen nicht eigentlich den SchematicSelectorInjectable?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +73,4 @@
public SchematicSelector(Player player, SelectorTarget target, SchematicSelectorInjectable injectable, Consumer<SchematicNode> callback) {
this(player, target, callback);
this.useHooks = true;
Besitzer

useHooks ist eine ganz schlechte Variable, weil du dafür, dass du diese Variable hast, jede menge ifs zur Verhaltensänderung brauchst. Wie wäre es damit, immer einen default-SchematicSelectorInjectable zu instanziieren? Dann brauchst du kein useHooks und kannst dir auch die ganzen ifs sparen.

useHooks ist eine ganz schlechte Variable, weil du dafür, dass du diese Variable hast, jede menge ifs zur Verhaltensänderung brauchst. Wie wäre es damit, immer einen default-SchematicSelectorInjectable zu instanziieren? Dann brauchst du kein useHooks und kannst dir auch die ganzen ifs sparen.
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +84,4 @@
openList(null, publicMode == PublicMode.PUBLIC_ONLY);
}
private void openList(SchematicNode parent, boolean publics) {
Besitzer

Statt überall publics als Parameter durchzutragen: Wie wäre es damit, im Falle einer Publicauflistung den SteamwarUser auf Public (aka id 0) zu setzen? Ein Parameter und viele Fallunterscheidungen weniger, oder?

Statt überall publics als Parameter durchzutragen: Wie wäre es damit, im Falle einer Publicauflistung den SteamwarUser auf Public (aka id 0) zu setzen? Ein Parameter und viele Fallunterscheidungen weniger, oder?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +85,4 @@
}
private void openList(SchematicNode parent, boolean publics) {
List<SchematicNode> nodes = filter.isFilter()?getFilteredSchematics(publics):getSchematicList(publics, parent);
Besitzer

Warum hier unterscheiden und nicht vereinfachen, indem immer gefiltert (ggf. mit leerem Filter) wird?

Warum hier unterscheiden und nicht vereinfachen, indem immer gefiltert (ggf. mit leerem Filter) wird?
Autor
Besitzer

Beim Filtern werden Alle Schematics gefiltert, was es etwas unnötig machen würde für normales anzeigen von Ordnern, alle Schematics zu laden.

Beim Filtern werden Alle Schematics gefiltert, was es etwas unnötig machen würde für normales anzeigen von Ordnern, alle Schematics zu laden.
@ -0,0 +103,4 @@
list.add(renderItem(node));
}
SWListInv<SchematicNode> inv = new SWListInv<>(player, MessageFormat.format(title==null?DEFAULT_TITLE:title.apply(player), target.target.getName(player), filter.getName().isEmpty()?(parent == null?"/":parent.generateBreadcrumbs(user)):filter.getName()), false, list, (clickType, node) -> handleClick(publics, node, parent));
Besitzer

Statt title==null? evtl. title defaultmäßig = () -> DEFAULT_TITLE; setzen?

Statt title==null? evtl. title defaultmäßig = () -> DEFAULT_TITLE; setzen?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +160,4 @@
name = name.replace(filter.getName(), "§e" + filter.getName() + "§7");
}
SWItem item = new SWItem(m, name, Collections.singletonList(node.isDir() ? ("§9" + Core.MESSAGE.parse("SCHEM_SELECTOR_DIR", player)) : "§7" + node.getSchemtype().name()), !node.isDir() && !node.getSchemtype().writeable(), click -> {
Besitzer

Eigentlich gehören auch ColorCodes in die Messages mit rein... Weil du kannst dir nicht sicher sein, dass die Farbe in einem anderen Kulturraum nicht eine andere Bedeutung hat...

Eigentlich gehören auch ColorCodes in die Messages mit rein... Weil du kannst dir nicht sicher sein, dass die Farbe in einem anderen Kulturraum nicht eine andere Bedeutung hat...
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +369,4 @@
return new SelectorTarget(Target.SCHEMATIC_TYPE, type, rank);
}
public static class SelectorTarget {
Besitzer

Nachdem alles private im Inneren ist, macht das keinerlei Sinn, das public zu stellen

Nachdem alles private im Inneren ist, macht das keinerlei Sinn, das public zu stellen
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +375,4 @@
private final SchematicType type;
private final int rank;
private SelectorTarget(Target target, SchematicType type, int rank) {
Besitzer

Hast du hierfür nicht deinen schönen @AllArgsConstructor?

Hast du hierfür nicht deinen schönen @AllArgsConstructor?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +391,4 @@
@Getter
private String rawName;
@Getter
Besitzer

Die Klasse ist private. Wozu getter?

Die Klasse ist private. Wozu getter?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +394,4 @@
@Getter
private boolean dirs;
public String getName(Player player) {
Besitzer

Klasse private, wozu public?

Klasse private, wozu public?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +406,4 @@
private boolean filter;
private String name = "";
Besitzer

Warum bei name mit "", bei allen anderen Werten aber mit null?

Warum bei name mit "", bei allen anderen Werten aber mit null?
Chaoscaot markierte diese Unterhaltung als gelöst
@ -0,0 +422,4 @@
public boolean matches(SchematicNode node) {
boolean matches = true;
if(!name.isEmpty()) {
if(!node.getName().contains(name)) {
Besitzer

Bitte die geschachtelten ifs zusammenfassen.

Bitte die geschachtelten ifs zusammenfassen.
Chaoscaot markierte diese Unterhaltung als gelöst
Chaoscaot hat 1 Commit 2021-11-27 10:13:24 +01:00 hinzugefügt
Fix Language & more PR Stuff
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
cf9f2a2375
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-27 10:34:57 +01:00 hinzugefügt
Fixing...
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
82421d9544
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-27 10:40:58 +01:00 hinzugefügt
More Language
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
becee281f6
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat ein Review von Lixfel 2021-11-27 10:41:14 +01:00 angefragt
Chaoscaot hat 1 Commit 2021-11-27 11:16:20 +01:00 hinzugefügt
Fix Default
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
f15e051d50
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-27 11:17:00 +01:00 hinzugefügt
Fix Default
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
117277c926
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-27 15:12:59 +01:00 hinzugefügt
Fix Default
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
e73ae9210c
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Lixfel hat die Änderungen 2021-11-30 16:58:41 +01:00 genehmigt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Bitte noch den Code aus dem Package util in das Package inventory verschieben (denn da gehört der Code eigentlich hin).

Ansonsten schaut der Code jetzt deutlich angenehmer zu lesen aus, ich kann keine Korrektheit garantieren, aber zumindest sollte er jetzt deutlich angenehmer zu warten sein.

Bitte noch den Code aus dem Package util in das Package inventory verschieben (denn da gehört der Code eigentlich hin). Ansonsten schaut der Code jetzt deutlich angenehmer zu lesen aus, ich kann keine Korrektheit garantieren, aber zumindest sollte er jetzt deutlich angenehmer zu warten sein.
Chaoscaot hat 1 Commit 2021-11-30 18:01:16 +01:00 hinzugefügt
Move Files
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
4d40d1aa46
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Chaoscaot hat 1 Commit 2021-11-30 18:09:25 +01:00 hinzugefügt
Add Command Mapper and some more Functions
Alle Prüfungen waren erfolgreich
SteamWarCI Build successful
646d9f7dab
Signed-off-by: Chaoscaot <chaoscaot444@gmail.com>
Lixfel hat Commit 146c97ff7a in master 2021-12-02 08:38:43 +01:00 gemerged
Lixfel löschte die Branch schematic_selector 2021-12-02 08:38:44 +01:00
Dieses Repo ist archiviert. Du kannst Pull-Requests nicht kommentieren.
Keine Beschreibung angegeben.