12
1

HellsBells #275

Manuell gemergt
Lixfel hat 19 Commits von Hells_bells nach master 2021-06-25 07:17:45 +02:00 zusammengeführt
Mitglied
Keine Beschreibung angegeben.
Zeanon hat 13 Commits 2021-05-14 16:57:57 +02:00 hinzugefügt
YoyoNow hat den Titel von Hells_bells zu HellsBells 2021-05-15 17:19:02 +02:00 geändert
Lixfel hat 2021-05-16 21:10:45 +02:00 Änderungen angefragt
@ -106,6 +107,9 @@ public class FightSystem extends JavaPlugin {
new RankedPlayerLeftWincondition();
new WinconditionPercentTimeout();
//noinspection InstantiationOfUtilityClass
Besitzer

Bitte rausnehmen (sollte auch nicht nötig sein?!?)

Bitte rausnehmen (sollte auch nicht nötig sein?!?)
Zeanon markierte diese Unterhaltung als gelöst
@ -0,0 +23,4 @@
public static final Random random = new Random();
private enum Direction {
Besitzer

Du kannst dafür nicht BlockFace verwenden?

Du kannst dafür nicht BlockFace verwenden?
Besitzer

In der 1.8 gibt es BlockFace mit dem toVector intern noch nicht.

In der 1.8 gibt es BlockFace mit dem toVector intern noch nicht.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +71,4 @@
@Override
public void enable() {
if (event != null) {
event.terminate();
Besitzer

In der enable() ist sichergestellt, dass es noch nie ausgeführt wurde oder vorher die disable() ausgeführt wurde, die Überprüfung sollte demnach unnötig sein.

In der enable() ist sichergestellt, dass es noch nie ausgeführt wurde oder vorher die disable() ausgeführt wurde, die Überprüfung sollte demnach unnötig sein.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +79,4 @@
@Override
public void disable() {
if (event != null) {
Besitzer

Es ist garantiert, dass vor der disable() die enable() ausgeführt wurde.

Es ist garantiert, dass vor der disable() die enable() ausgeführt wurde.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +87,4 @@
}.register();
}
private static class HellsBellsEvent {
Besitzer

Diese Unterklasse scheint mir ziemlich unnötig, die Inhalte dieser Klasse sollten in die übergeordnete Klasse eingebaut werden können.

Diese Unterklasse scheint mir ziemlich unnötig, die Inhalte dieser Klasse sollten in die übergeordnete Klasse eingebaut werden können.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +110,4 @@
startCountdown();
}
public void terminate() {
Besitzer

init() und terminate() lesen sich mir wie eine enable() und disable() eines StateDependent...

init() und terminate() lesen sich mir wie eine enable() und disable() eines StateDependent...
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +111,4 @@
}
public void terminate() {
current = State.NONE;
Besitzer

Wird der State NONE wirklich benötigt?

Wird der State NONE wirklich benötigt?
Zeanon markierte diese Unterhaltung als gelöst
@ -0,0 +128,4 @@
}
currentDrops++;
currentCountdown = new HellsBellsCountdown(current.MIN_TIME + random.nextInt(current.MAX_TIME - current.MIN_TIME), SWSound.BLOCK_NOTE_BASS, true);
Besitzer

Gibt es nicht random.nextInt(MIN, MAX)? (kA)

Gibt es nicht random.nextInt(MIN, MAX)? (kA)
Besitzer

Nein leider bietet die normale Random implementation dies nicht an.

Nein leider bietet die normale Random implementation dies nicht an.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +139,4 @@
AtomicInteger length = new AtomicInteger(10 + random.nextInt((direction.zLength ? zLength : xLength) - 10));
int width = 5 + random.nextInt(5);
int xOffset = getWidthStart(direction.zLength ? xLength : zLength, direction.zLength ? length.get() : width);
int zOffset = getLengthStart(direction.zLength ? zLength : xLength, direction.zLength ? width : length.get());
Besitzer

Könnte man statt diesen Ifs nicht einfach eine Methode in der Direction machen, die dann den passenden Wert zurückgibt?

Könnte man statt diesen Ifs nicht einfach eine Methode in der Direction machen, die dann den passenden Wert zurückgibt?
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +145,4 @@
Point redStart;
Point blueStart;
if (direction == Direction.NORTH || direction == Direction.WEST) {
Besitzer

Auch hier müsste das zu einer Methode in Direction umwandelbar sein.

Auch hier müsste das zu einer Methode in Direction umwandelbar sein.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +153,4 @@
blueStart = new Point(Config.BluePasteRegion.getMaxX() - xOffset, Config.BlueExtendRegion.getMaxY() + yOffset, Config.BluePasteRegion.getMaxZ() - zOffset);
}
new BukkitRunnable() {
Besitzer

Üblicherweise verwenden wir eher Bukkit...getScheduler().runTaskTimer(Plugin, lambda-Methode, Zeit(en))

Üblicherweise verwenden wir eher Bukkit...getScheduler().runTaskTimer(Plugin, lambda-Methode, Zeit(en))
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +189,4 @@
private class HellsBellsCountdown extends Countdown {
public HellsBellsCountdown(int time, SWSound sound, boolean level) {
Besitzer

Ich glaube, du verwendest immer dieselben Parameter (bis auf die variable Zeit), daher kannst du den Sound und Level direkt im Konstruktor setzen und musst das nicht übergeben...

Ich glaube, du verwendest immer dieselben Parameter (bis auf die variable Zeit), daher kannst du den Sound und Level direkt im Konstruktor setzen und musst das nicht übergeben...
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +206,4 @@
}
}
private static class Point {
Besitzer

Warum geht es nicht, Location oder Vector zu verwenden?

Warum geht es nicht, Location oder Vector zu verwenden?
Autor
Mitglied

Weil Location und Vektor zu mehr undurchsichtigem code führt

Weil Location und Vektor zu mehr undurchsichtigem code führt
YoyoNow markierte diese Unterhaltung als gelöst
Lixfel hat 2021-05-16 21:11:03 +02:00 Änderungen angefragt
@ -0,0 +1,271 @@
package de.steamwar.fightsystem.event;
Besitzer

License fehlt.

License fehlt.
YoyoNow markierte diese Unterhaltung als gelöst
Zeanon hat 1 Commit 2021-05-18 19:58:36 +02:00 hinzugefügt
YoyoNow hat 1 Commit 2021-05-18 20:03:53 +02:00 hinzugefügt
Zeanon hat 1 Commit 2021-05-19 10:19:36 +02:00 hinzugefügt
Lixfel hat 2021-05-23 12:04:50 +02:00 Änderungen angefragt
@ -0,0 +42,4 @@
public static final Random random = new Random();
private enum Direction {
Besitzer

Subklassen bitte nach dem Inhalt der Hauptklasse.

Subklassen bitte nach dem Inhalt der Hauptklasse.
@ -0,0 +48,4 @@
EAST(1, 0, 0),
WEST(-1, 0, 0);
static final Direction[] DIRECTIONS = values();
Besitzer

Warum das nochmal extra vorhalten? In getRandom kann ja auch values() aufgerufen werden...

Warum das nochmal extra vorhalten? In getRandom kann ja auch values() aufgerufen werden...
Besitzer

values() auf einem Enum erzeugt immer eine kopie der Enum Elemente intern, daher recht inefficient.

values() auf einem Enum erzeugt immer eine kopie der Enum Elemente intern, daher recht inefficient.
Besitzer

Ich finde, es ist ineffizienter, eine Kopie ständig im RAM zu halten, als alle x Minuten mal eine neue Kopie des 4! Elemente großen Array anzulegen. Ist zudem kürzer und einfacher, einfach neu zu kopieren.

Ich finde, es ist ineffizienter, eine Kopie ständig im RAM zu halten, als alle x Minuten mal eine neue Kopie des 4! Elemente großen Array anzulegen. Ist zudem kürzer und einfacher, einfach neu zu kopieren.
@ -0,0 +93,4 @@
}
public HellsBells() {
new StateDependent(Winconditions.HELLS_BELLS, FightState.Running) {
Besitzer

Das StateDependent sollte nur die Sachen (de-)aktivieren, nicht das ganze Behaviour innehaben. Um die Nestung zu verringern: Nimm den ganzen Code bis auf die Enable und Disable und bewege den in die HellsBells-Klasse. (Also die übergeordnete)

Das StateDependent sollte nur die Sachen (de-)aktivieren, nicht das ganze Behaviour innehaben. Um die Nestung zu verringern: Nimm den ganzen Code bis auf die Enable und Disable und bewege den in die HellsBells-Klasse. (Also die übergeordnete)
Zeanon markierte diese Unterhaltung als gelöst
@ -0,0 +135,4 @@
Point redStart;
Point blueStart;
if (direction.isNorthOrWest()) {
Besitzer

Diesen if-Block kann man denke ich vereinfachen, wenn man stattdessen in den Assignments mit dem conditional-Operator arbeitet ?:.

Diesen if-Block kann man denke ich vereinfachen, wenn man stattdessen in den Assignments mit dem conditional-Operator arbeitet ?:.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +145,4 @@
currentDropping = Bukkit.getScheduler().runTaskTimer(FightSystem.getPlugin(), () -> {
for (int w = 0; w < width; w++) {
if (direction.isNorthOrWest()) {
Besitzer

Stat dem If-Block mit Subtract and add: Alle Werte mit int factor = direction.isNorthOrWest() ? 1 : -1; multiplizieren.

Stat dem If-Block mit Subtract and add: Alle Werte mit `int factor = direction.isNorthOrWest() ? 1 : -1;` multiplizieren.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +204,4 @@
}.register();
}
private static class Point {
Besitzer

Ich weiß nicht, aber ich glaube, dafür, dass an einer Stelle mal ein subtract und ein add ausgeführt wird (generalisierbar mit * +-1) ist es etwas overkill, eine eigene Klasse dafür vorzuhalten, selbst wenn Location in der 1.8 (glaube war der Grund) eine .add o.ä. Methode bereitstellt, ist etwas overkill. Dann hat man halt beim Aufruf etwas (mehr) Arithmetik.

Ich weiß nicht, aber ich glaube, dafür, dass an einer Stelle mal ein subtract und ein add ausgeführt wird (generalisierbar mit * +-1) ist es etwas overkill, eine eigene Klasse dafür vorzuhalten, selbst wenn Location in der 1.8 (glaube war der Grund) eine .add o.ä. Methode bereitstellt, ist etwas overkill. Dann hat man halt beim Aufruf etwas (mehr) Arithmetik.
Zeanon markierte diese Unterhaltung als gelöst
YoyoNow hat 1 Commit 2021-05-23 17:07:48 +02:00 hinzugefügt
Zeanon hat 1 Commit 2021-06-16 15:39:47 +02:00 hinzugefügt
Lixfel hat 2021-06-20 13:49:09 +02:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Schaut jetzt schon mal ganz gut aus, in dieser Fassung getestet?

Schaut jetzt schon mal ganz gut aus, in dieser Fassung getestet?
@ -0,0 +138,4 @@
}.register();
}
class HellsBellsCountdown extends Countdown {
Besitzer

Kann glaube noch private static sein.

Kann glaube noch private static sein.
Zeanon hat 1 Commit 2021-06-21 19:55:51 +02:00 hinzugefügt
Lixfel hat die Änderungen 2021-06-25 07:17:38 +02:00 genehmigt
Lixfel hat Commit cae51875fc in master 2021-06-25 07:17:45 +02:00 manuell gemerged
Lixfel löschte die Branch Hells_bells 2021-06-25 07:17:58 +02:00
Anmelden, um an der Diskussion teilzunehmen.
Keine Beschreibung angegeben.