SteamWar/BauSystem
Archiviert
13
0

Add basic sleep command to ScriptSystem #142

Manuell gemergt
YoyoNow hat 16 Commits von ScriptSleep nach master 2020-12-16 12:22:20 +01:00 zusammengeführt
Besitzer
Keine Beschreibung angegeben.
Lixfel hat 2020-12-15 21:16:38 +01:00 Änderungen angefragt
@ -38,3 +42,1 @@
public void onLeftClick(PlayerInteractEvent event) {
if(event.getAction() != Action.LEFT_CLICK_AIR && event.getAction() != Action.LEFT_CLICK_BLOCK)
return;
private Map<BookMeta, Script> scriptMap = new HashMap<>();
Besitzer

Wozu musst du dir die Scripte speichern? Ist glaube ich unnötig.

Wozu musst du dir die Scripte speichern? Ist glaube ich unnötig.
@ -49,3 +46,1 @@
for(String page : meta.getPages()){
for(String command : page.split("\n")){
if (command.startsWith("#")) continue;
private BookMeta bookMeta;
Besitzer

Wozu braucht das Skript die BookMeta?

Wozu braucht das Skript die BookMeta?
@ -50,2 +46,2 @@
for(String command : page.split("\n")){
if (command.startsWith("#")) continue;
private BookMeta bookMeta;
private List<String> commands = new ArrayList<>();
Besitzer

LinkedList wäre bei Arbeit mit entfernen besser geeignet.

LinkedList wäre bei Arbeit mit entfernen besser geeignet.
Autor
Besitzer

Ich entferne ja absolut nichts.
YoyoNow:

Dies ist extra so, um weitere Features einzubauen, die das Turing Complete machen würden.

Ich entferne ja absolut nichts. YoyoNow: > Dies ist extra so, um weitere Features einzubauen, die das Turing Complete machen würden.
@ -51,1 +46,3 @@
if (command.startsWith("#")) continue;
private BookMeta bookMeta;
private List<String> commands = new ArrayList<>();
private int uses = 0;
Besitzer

Wozu müssen die uses gespeichert werden? Fire and forget!

Wozu müssen die uses gespeichert werden? Fire and forget!
@ -52,0 +58,4 @@
}
}
public void keep() {
Besitzer

KISS: Lass das weg.

KISS: Lass das weg.
@ -52,0 +62,4 @@
uses++;
}
public void free() {
Besitzer

KISS: Lass das weg.

KISS: Lass das weg.
@ -52,0 +71,4 @@
}
private class ScriptExecutor {
Besitzer

ScriptExecutor mit Script mergen, gibt keinen Grund das zu trennen.

ScriptExecutor mit Script mergen, gibt keinen Grund das zu trennen.
@ -52,0 +75,4 @@
private Player player;
private Script script;
private int index = 0;
Besitzer

Nicht mit einem index arbeiten, sondern die abgearbeiteten Commands aus der Liste löschen.

Nicht mit einem index arbeiten, sondern die abgearbeiteten Commands aus der Liste löschen.
Autor
Besitzer

Dies ist extra so, um weitere Features einzubauen, die das Turing Complete machen würden.

Dies ist extra so, um weitere Features einzubauen, die das Turing Complete machen würden.
@ -52,0 +87,4 @@
script.keep();
}
public void start() {
Besitzer

Verwendest du nie, und solltest du bei Arbeit nur mit Skript nicht brauchen (im Konstruktor einfach starten)

Verwendest du nie, und solltest du bei Arbeit nur mit Skript nicht brauchen (im Konstruktor einfach starten)
@ -52,0 +91,4 @@
resume();
}
private void resume() {
Besitzer

Fehlend: Überprüfung, ob der Spieler überhaupt noch online ist, ansonsten könnten da schnell Fehler fliegen.

Fehlend: Überprüfung, ob der Spieler überhaupt noch online ist, ansonsten könnten da schnell Fehler fliegen.
@ -52,0 +95,4 @@
while (index < script.commands.size()) {
String command = script.commands.get(index++);
if (command.startsWith("sleep")) {
Besitzer

CaSeInDePeNdEnCe

CaSeInDePeNdEnCe
@ -52,0 +96,4 @@
String command = script.commands.get(index++);
if (command.startsWith("sleep")) {
String sleepTimeString = command.substring(5).trim();
Besitzer

Evtl eher mit String.split() arbeiten (vgl andere Commands)

Evtl eher mit String.split() arbeiten (vgl andere Commands)
@ -52,0 +99,4 @@
String sleepTimeString = command.substring(5).trim();
int sleepTime = 1;
try {
Integer.parseInt(sleepTimeString);
Besitzer

Ergebnis wird ignoriert.

Ergebnis wird ignoriert.
Autor
Besitzer

Auch dies sollte behoben sein, das habe ich einfach vergessen.

Auch dies sollte behoben sein, das habe ich einfach vergessen.
Lixfel hat 2020-12-16 09:16:32 +01:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Wann wird der Tracer gefixt?

Wann wird der Tracer gefixt?
@ -38,3 +42,1 @@
public void onLeftClick(PlayerInteractEvent event) {
if(event.getAction() != Action.LEFT_CLICK_AIR && event.getAction() != Action.LEFT_CLICK_BLOCK)
return;
private static class ScriptExecutor {
Besitzer

Subklassen kommen üblicherweise erst nach den Methoden der eigenen Klassen im Quelltext

Subklassen kommen üblicherweise erst nach den Methoden der eigenen Klassen im Quelltext
@ -43,2 +44,2 @@
if(item == null || isNoBook(item) || item.getItemMeta() == null)
return;
private Player player;
private List<String> commands = new ArrayList<>();
Besitzer

player & commands sind final

player & commands sind final
@ -52,0 +59,4 @@
private void resume() {
if (!player.isOnline()) {
player = null;
Besitzer

Du musst hier nix auf null setzen, nach Ablauf der Funktion wird dieses Objekt sowieso nirgends mehr referenziert.

Du musst hier nix auf null setzen, nach Ablauf der Funktion wird dieses Objekt sowieso nirgends mehr referenziert.
@ -52,0 +68,4 @@
String command = commands.get(index++);
String[] strings = command.split(" ");
if (strings.length > 0) {
Besitzer

Befehle ohne Leerzeichen werden nicht mehr ausgeführt.

Befehle ohne Leerzeichen werden nicht mehr ausgeführt.
@ -52,0 +73,4 @@
int sleepTime = 1;
if (strings.length > 1) {
try {
sleepTime = Integer.parseInt(strings[1]);
Besitzer

Negative und 0 sleepTimes incoming

Negative und 0 sleepTimes incoming
@ -52,0 +75,4 @@
try {
sleepTime = Integer.parseInt(strings[1]);
} catch (NumberFormatException e) {
// Ignored
Besitzer

Ne Fehlermeldung wäre schon ganz nice, wenn man da was nichtnumerisches angegeben hat.

Ne Fehlermeldung wäre schon ganz nice, wenn man da was nichtnumerisches angegeben hat.
@ -52,0 +77,4 @@
} catch (NumberFormatException e) {
// Ignored
}
}
Besitzer

Hast du das überhaupt mal getestet (und trotzdem als fertigen PR markiert)? Der einzige Befehl, den das Skriptsystem so interpretiert, ist dein Sleep-Command!

Hast du das überhaupt mal getestet (und trotzdem als fertigen PR markiert)? Der einzige Befehl, den das Skriptsystem so interpretiert, ist dein Sleep-Command!
@ -62,0 +106,4 @@
event.setCancelled(true);
Player player = event.getPlayer();
BookMeta meta = (BookMeta) item.getItemMeta();
Besitzer

Das casten und getPlayer() kann man auch beim Aufruf des Konstruktors machen, kein Bedarf für diese zusätzlichen Throw-Away-Referenzen

Das casten und getPlayer() kann man auch beim Aufruf des Konstruktors machen, kein Bedarf für diese zusätzlichen Throw-Away-Referenzen
Autor
Besitzer

Soll es so sein, dass Scripte auch beim Öffnen ausgeführt werden?

Soll es so sein, dass Scripte auch beim Öffnen ausgeführt werden?
Besitzer

Beim Öffnen des Buches? Nein, eigentlich nicht, allerdings scheint das trotzdem irgendwie als LEFT_CLICK_AIR oder LEFT_CLICK_BLOCK gewertet zu werden. Wenn du da ein besseres Event findest: Go for it!

Beim Öffnen des Buches? Nein, eigentlich nicht, allerdings scheint das trotzdem irgendwie als LEFT_CLICK_AIR oder LEFT_CLICK_BLOCK gewertet zu werden. Wenn du da ein besseres Event findest: Go for it!
Autor
Besitzer

Wenn du die Luft rechts klickst dann wird dies als left click gezählt. Auf einem Block tut es einwandfrei

Wenn du die Luft rechts klickst dann wird dies als left click gezählt. Auf einem Block tut es einwandfrei
Lixfel hat 2020-12-16 09:40:54 +01:00 Änderungen angefragt
@ -74,0 +73,4 @@
this.player = player;
for(String page : bookMeta.getPages()) {
for (String command : page.split("\n")) {
Besitzer

Bitte auch leere Zeilen rausfiltern (kein gültiger Befehl).

Bitte auch leere Zeilen rausfiltern (kein gültiger Befehl).
@ -74,0 +89,4 @@
while (index < commands.size()) {
String command = commands.get(index++);
if (command.contains(" ")) {
Besitzer

Bugs inc! Prüfe bitte ZUERST, ob der Befehl mit SlEEp anfängt, und dann kümmere dich um die Sleep-abarbeitung, und nicht zuerst, ob da ein space drinist!

Bugs inc! Prüfe bitte ZUERST, ob der Befehl mit SlEEp anfängt, und dann kümmere dich um die Sleep-abarbeitung, und nicht zuerst, ob da ein space drinist!
Autor
Besitzer

Und wie soll ich bitte case insensitive das ganze checken mit einer regex?

Und wie soll ich bitte case insensitive das ganze checken mit einer regex?
Besitzer

command.toLowerCase().startsWith()

command.toLowerCase().startsWith()
Autor
Besitzer

Auch dies sollte behoben sein!

Auch dies sollte behoben sein!
@ -74,0 +95,4 @@
if (strings[0].equalsIgnoreCase("sleep")) {
int sleepTime = 1;
if (strings.length > 1) {
try {
Besitzer

Woho! Die 9. offene Klammer! Bester Programmierstil!

Woho! Die 9. offene Klammer! Bester Programmierstil!
Autor
Besitzer

Hab ich behoben

Hab ich behoben
@ -74,0 +106,4 @@
}
}
Bukkit.getScheduler().runTaskLater(BauSystem.getPlugin(), this::resume, sleepTime);
break;
Besitzer

return;

Dann ist der Programmfluss klarer.

return; Dann ist der Programmfluss klarer.
@ -74,0 +113,4 @@
PlayerCommandPreprocessEvent preprocessEvent = new PlayerCommandPreprocessEvent(player, "/" + command);
Bukkit.getServer().getPluginManager().callEvent(preprocessEvent);
if (preprocessEvent.isCancelled()) {
Besitzer

Das macht wenn schon der PreprocessEventHandler, lass das hier raus!

Das macht wenn schon der PreprocessEventHandler, lass das hier raus!
Autor
Besitzer

Sollten wir nicht ab dem ersten Befehl der nicht ausführbar ist abbrechen?

Sollten wir nicht ab dem ersten Befehl der nicht ausführbar ist abbrechen?
Besitzer

Dann machst du einfach ein Return, und erzählst dem User keinen Schmarrn!

Das problem ist: Ob der eigentliche Befehl ausgeführt wurde (das kommt ja erst drunter, das ist nur die Rechteüberprüfung für WE-Befehle (mehr oder weniger)) können wir als Befehlsdispatcher gar nicht feststellen. Daher: Das Skript läuft immer durch, egal wie viele Befehle davon "schiefgehen"

Dann machst du einfach ein Return, und erzählst dem User keinen Schmarrn! Das problem ist: Ob der eigentliche Befehl ausgeführt wurde (das kommt ja erst drunter, das ist nur die Rechteüberprüfung für WE-Befehle (mehr oder weniger)) können wir als Befehlsdispatcher gar nicht feststellen. Daher: Das Skript läuft immer durch, egal wie viele Befehle davon "schiefgehen"
Besitzer

YoyoNow, deshalb: Die Nutzung eines anderen Events wäre ggf. besser (wenn es ein passenderes gibt)

YoyoNow, deshalb: Die Nutzung eines anderen Events wäre ggf. besser (wenn es ein passenderes gibt)
Autor
Besitzer

Ok ich schaffe es, dass ich alles, was mit der Luft zu tun hat nun wegcancel? richtig komisch.

Ok ich schaffe es, dass ich alles, was mit der Luft zu tun hat nun wegcancel? richtig komisch.
Autor
Besitzer

Ich jetzt bekomme ich sowohl ein RightClickAir und LeftClickAir wenn ich RightClickAir ausführe. Das ist absolut komisch, was dahinter steckt!

Ich jetzt bekomme ich sowohl ein RightClickAir und LeftClickAir wenn ich RightClickAir ausführe. Das ist absolut komisch, was dahinter steckt!
Besitzer

Von der Abstrahierung des ScriptCommands halte ich nichts, da es neben sleep keinen Skriptcommand gibt und auch keiner geplant ist. Verzweigungen & Schleifen sind KEINE Commands, diese benötigen nämlich andere Datenstrukturen. (Ansonsten kommst du schnell in Probleme durch verzweigten Verzweigungskonstrukten.

Bitte wieder ausbauen, die Komplexität tut dem Skriptsystem nicht gut.

Von der Abstrahierung des ScriptCommands halte ich nichts, da es neben sleep keinen Skriptcommand gibt und auch keiner geplant ist. Verzweigungen & Schleifen sind KEINE Commands, diese benötigen nämlich andere Datenstrukturen. (Ansonsten kommst du schnell in Probleme durch verzweigten Verzweigungskonstrukten. Bitte wieder ausbauen, die Komplexität tut dem Skriptsystem nicht gut.
Autor
Besitzer

Ich hätte damit noch ein 'exit' befehl eingebaut, der einfach die exection stoppt. Deswegen hätte ich das so gebaut.

Ich hätte damit noch ein 'exit' befehl eingebaut, der einfach die exection stoppt. Deswegen hätte ich das so gebaut.
Besitzer

Overhead ist für 2 Befehle zu groß (mMn - meiner Meinung nach)

Overhead ist für 2 Befehle zu groß (mMn - meiner Meinung nach)
Autor
Besitzer

Ich glaube dann sollte es jetzt nach deinen ansprüchen sein mit dem Sleep Befehl!

Ich glaube dann sollte es jetzt nach deinen ansprüchen sein mit dem Sleep Befehl!
Lixfel hat 2020-12-16 11:19:25 +01:00 Änderungen angefragt
@ -35,3 +41,3 @@
public class ScriptListener implements Listener {
@EventHandler
private static final String scriptPrefix = "§eScriptSystem§8» §7";
Besitzer

Nutze wenn bitte den BauSystem-Prefix

Nutze wenn bitte den BauSystem-Prefix
Autor
Besitzer

Warum das, das ist ja explizit das ScripSystem

Warum das, das ist ja explizit das ScripSystem
Besitzer

Nein, das ist hier immer noch das BauSystem.

Nein, das ist hier immer noch das BauSystem.
@ -37,1 +43,3 @@
@EventHandler
private static final String scriptPrefix = "§eScriptSystem§8» §7";
private Set<Player> playerSet = new HashSet<>();
Besitzer

Sollte ein Spieler den Server verlassen oder das Item in der Hand wechseln, musst du ihn auch hier rauswerfen. Evtl. ist es einfacher, alle Tick das zu clearen? (Je nachdem, wie die zeitlich getimet sind?)

Sollte ein Spieler den Server verlassen oder das Item in der Hand wechseln, musst du ihn auch hier rauswerfen. Evtl. ist es einfacher, alle Tick das zu clearen? (Je nachdem, wie die zeitlich getimet sind?)
@ -38,0 +44,4 @@
private Set<Player> playerSet = new HashSet<>();
@EventHandler(priority = EventPriority.HIGH)
Besitzer

Lass das HIGH da raus, das hat da nix verloren.

Lass das HIGH da raus, das hat da nix verloren.
Autor
Besitzer

Das muss so, weil bei Normal in den RightClickAir nicht mehr bekomme. Dies habe ich getestet. Dies ist ein Bukkit Feature

Das muss so, weil bei Normal in den RightClickAir nicht mehr bekomme. Dies habe ich getestet. Dies ist ein Bukkit Feature
Besitzer

Dafür möchte ich ne Quelle, weil das scheint mir aufgrunddessen, wie Events abgearbeitet werden, absolut unwahrscheinlich.

Dafür möchte ich ne Quelle, weil das scheint mir aufgrunddessen, wie Events abgearbeitet werden, absolut unwahrscheinlich.
Autor
Besitzer

https://bukkit.org/threads/right-click-air-not-working.103210/
Von Malikk, Sep 30, 2012
etwas weiter unten

https://bukkit.org/threads/right-click-air-not-working.103210/ Von Malikk, Sep 30, 2012 etwas weiter unten
Besitzer

Das ist halt auch nur eine Meinung, ne Ursachenbegründung ist das nicht.

Das ist halt auch nur eine Meinung, ne Ursachenbegründung ist das nicht.
@ -38,2 +47,3 @@
@EventHandler(priority = EventPriority.HIGH)
public void onLeftClick(PlayerInteractEvent event) {
if(event.getAction() != Action.LEFT_CLICK_AIR && event.getAction() != Action.LEFT_CLICK_BLOCK)
if (event.getAction() != Action.LEFT_CLICK_AIR && event.getAction() != Action.LEFT_CLICK_BLOCK) {
Besitzer

Umfassend getestet mit allen möglichen Aktionen, unterbindet es alle ungewollten aktivierungen und lässt noch alle gewollten Aktivierungen durch?

Derzeit dürfte das Bugs geben, wenn du z.B. zwischendurch durch ein Tripwire läufst oder ne Druckplatte drückst.

Umfassend getestet mit allen möglichen Aktionen, unterbindet es alle ungewollten aktivierungen und lässt noch alle gewollten Aktivierungen durch? Derzeit dürfte das Bugs geben, wenn du z.B. zwischendurch durch ein Tripwire läufst oder ne Druckplatte drückst.
@ -41,3 +57,4 @@
}
ItemStack item = event.getItem();
if(item == null || isNoBook(item) || item.getItemMeta() == null)
Besitzer

Bitte wenn erst diese simple überprüfung machen, und DANN erst die ungewollten aktivierungen unterbinden

Bitte wenn erst diese simple überprüfung machen, und DANN erst die ungewollten aktivierungen unterbinden
@ -52,0 +86,4 @@
for(String page : bookMeta.getPages()) {
for (String command : page.split("\n")) {
if (command.startsWith("#")) continue;
if (command.trim().isEmpty()) continue;
Besitzer

Bitte ifs verodern und zusammenfassen!

Bitte ifs verodern und zusammenfassen!
@ -71,0 +124,4 @@
return fullCommand.substring(command.length() + 1).split(" ");
}
private static void sleepCommand(ScriptExecutor scriptExecutor, String[] args) {
Besitzer

Da wäre es Sinnvoller, einfach den player zu übergeben, was will sleep mit dem ScriptExecutor?

Da wäre es Sinnvoller, einfach den player zu übergeben, was will sleep mit dem ScriptExecutor?
Autor
Besitzer

Ihn resumen. ganz unten

Ihn resumen. ganz unten
@ -71,0 +130,4 @@
try {
sleepTime = Integer.parseInt(args[0]);
if (sleepTime <= 0) {
scriptExecutor.player.sendMessage(scriptPrefix + "Sleep kleiner gleich 0, default 1 GameTick");
Besitzer

Das kannst du so keinem Spieler vorwerfen, bitte überarbeiten.

Das kannst du so keinem Spieler vorwerfen, bitte überarbeiten.
Autor
Besitzer

Wie sollte ich diese Nachricht denn dann überarbeiten

Wie sollte ich diese Nachricht denn dann überarbeiten
Besitzer

Hirn an und dem Nutzer eine für ihn informative Nachricht in einer ihm verständlichen Sprache, Art und Weise senden.

Hirn an und dem Nutzer eine für ihn informative Nachricht in einer ihm verständlichen Sprache, Art und Weise senden.
@ -71,0 +134,4 @@
sleepTime = 1;
}
} catch (NumberFormatException e) {
scriptExecutor.player.sendMessage(scriptPrefix + "Sleep ohne Zahl, default 1 GameTick");
Besitzer

Gleichfalls.

Gleichfalls.
Autor
Besitzer

Dann sollte dies nun auch behoben sein.

Dann sollte dies nun auch behoben sein.
Lixfel hat 2020-12-16 11:38:02 +01:00 Änderungen angefragt
@ -71,0 +133,4 @@
try {
sleepTime = Integer.parseInt(args[0]);
if (sleepTime <= 0) {
scriptExecutor.player.sendMessage(BauSystem.PREFIX + "Eine Sleep zeit von kleiner gleich 0 ist nicht erlaubt. Der default 1 Tick wird verwendet.");
Besitzer

Deutsch!

Deutsch!
@ -71,0 +137,4 @@
sleepTime = 1;
}
} catch (NumberFormatException e) {
scriptExecutor.player.sendMessage(BauSystem.PREFIX + "Eine Sleep zeit sollte keine Buchstaben oder sonstige Zeiten verwenden. Der default 1 Tick wird verwendet.");
Besitzer

Deutsch!

Deutsch!
Autor
Besitzer

Ich glaube das ist jetzt besseres Deutsch

Ich glaube das ist jetzt besseres Deutsch
Besitzer

Ne, nicht wirklich. lass mal sleep raus, und default interessiert den Nutzer auch nicht.

Ne, nicht wirklich. lass mal sleep raus, und default interessiert den Nutzer auch nicht.
Autor
Besitzer

So?

So?
Besitzer

sollte ist von der Formulierung nicht ganz richtig... => muss

Farbcodes nicht vergessen, ansonsten schaut das jetzt schon langsam gut aus.

sollte ist von der Formulierung nicht ganz richtig... => muss Farbcodes nicht vergessen, ansonsten schaut das jetzt schon langsam gut aus.
Autor
Besitzer

Gut dann habe ich das nun auch noch gemacht

Gut dann habe ich das nun auch noch gemacht
Lixfel hat 2020-12-16 12:03:43 +01:00 Änderungen angefragt
@ -71,0 +135,4 @@
sleepTime = 1;
}
} catch (NumberFormatException e) {
scriptExecutor.player.sendMessage(BauSystem.PREFIX + "§cDie Zeit muss nur aus Zahlen bestehen.");
Besitzer

darf

darf
Lixfel hat die Änderungen 2020-12-16 12:08:22 +01:00 genehmigt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Unbedingt nochmal vor Mergen testen.

Unbedingt nochmal vor Mergen testen.
YoyoNow hat diesen Pull-Request 2020-12-16 12:22:20 +01:00 geschlossen
YoyoNow löschte die Branch ScriptSleep 2020-12-16 12:22:35 +01:00
Dieses Repo ist archiviert. Du kannst Pull-Requests nicht kommentieren.
Keine Beschreibung angegeben.