SteamWar/FightSystem
Archiviert
13
1

RedstoneWincondition #274

Geschlossen
YoyoNow möchte 18 Commits von RedstoneWincondition nach master mergen
Besitzer
Keine Beschreibung angegeben.
YoyoNow hat 3 Commits 2021-05-09 15:37:20 +02:00 hinzugefügt
Lixfel hat 2021-05-12 20:52:25 +02:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Sr, das scheint mir einfach nur Low-Efford gecopy+pasted zu sein, kommt so nicht ins FightSystem.

Sr, das scheint mir einfach nur Low-Efford gecopy+pasted zu sein, kommt so nicht ins FightSystem.
@ -98,6 +98,9 @@ public class FightSystem extends JavaPlugin {
new WinconditionWaterTechKO();
new WinconditionPercentSystem();
new WinconditionRelativePercent();
if (Core.getVersion() >= 15) {
Besitzer

Der Aufwand, das ganze 1.12-Kompatibel zu machen, ist nicht so groß, bitte machen (da eigentlich alles im Fightsystem für alle Versionen ausgelegt ist). Und nicht den 1.14-Support vergessen.

Der Aufwand, das ganze 1.12-Kompatibel zu machen, ist nicht so groß, bitte machen (da eigentlich alles im Fightsystem für alle Versionen ausgelegt ist). Und nicht den 1.14-Support vergessen.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +32,4 @@
Set<Material> ignored = new HashSet<>();
for (String s : Config.IgnoredBlocks)
ignored.add(Material.valueOf(s));
ignoredBlocks = Collections.unmodifiableSet(ignored);
Besitzer

Richt mir ein bisschen nach Code-Duplication, bitte auslagern, dass der Code nur einmal da ist.

Richt mir ein bisschen nach Code-Duplication, bitte auslagern, dass der Code nur einmal da ist.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +45,4 @@
if (Config.ActiveWinconditions.contains(Winconditions.RELATIVE_REDSTONE_PERCENT)) {
printableWinconditions.add(this);
percentWincondition = this;
Bukkit.getPluginManager().registerEvents(this, FightSystem.getPlugin());
Besitzer

Bitte einen StateDependentListener verwenden.

Bitte einen StateDependentListener verwenden.
Autor
Besitzer

Kann ich aber nicht hier in der Klasse machen, weil 2 mal Extend wird wohl nichts!

Kann ich aber nicht hier in der Klasse machen, weil 2 mal Extend wird wohl nichts!
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +56,4 @@
@Override
public String getDisplay(FightTeam team) {
return team.getPrefix() + "Schaden: " + (Math.round(100.0 * getPercent(team)) / 100.0) + "%";
Besitzer

Liest sich wie Code-Duplication, evtl PercentWincondition PrintableWincondition extenden lassen und dann eine default-Implementierung im Interface (Formatierung dürfte für alle 3 Prozentsysteme gleich sein)

Liest sich wie Code-Duplication, evtl PercentWincondition PrintableWincondition extenden lassen und dann eine default-Implementierung im Interface (Formatierung dürfte für alle 3 Prozentsysteme gleich sein)
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +60,4 @@
}
@EventHandler
public void onBlockPlace(BlockPlaceEvent event) {
Besitzer

Den Listener kannst du weglassen, was möchtest du damit überhaupt? Es sind eh nur die zyklischen Prüfungen relevant.

Den Listener kannst du weglassen, was möchtest du damit überhaupt? Es sind eh nur die zyklischen Prüfungen relevant.
Autor
Besitzer

Nein, wenn ein Block plaziert wurde, darf man keine Prozente zurückbekommen.

Nein, wenn ein Block plaziert wurde, darf man keine Prozente zurückbekommen.
Besitzer

Dann wäre allerdings der explosionsbasierte Messansatz besser als der zeitbasiert prüfende.

Dann wäre allerdings der explosionsbasierte Messansatz besser als der zeitbasiert prüfende.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +70,4 @@
}
}
public static class TeamPercent extends StateDependent {
Besitzer

Der Code hier liest sich wie Copy+Paste aus der RelativeWincondition. Codeduplication bitte entfernen (aka generalisieren)

Der Code hier liest sich wie Copy+Paste aus der RelativeWincondition. Codeduplication bitte entfernen (aka generalisieren)
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +81,4 @@
super(Winconditions.RELATIVE_REDSTONE_PERCENT, FightState.Running);
this.team = team;
this.currentBlocks = 1;
register();
Besitzer

Fände es nicer, wenn hier ein StateDependentTask und ein OneShotStateDependent statt des extends StateDependent verwendet wird.

Fände es nicer, wenn hier ein StateDependentTask und ein OneShotStateDependent statt des extends StateDependent verwendet wird.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +115,4 @@
private int currentBlocks() {
// Entern active
if (!Config.EnterStages.isEmpty() && Config.EnterStages.get(0) >= Wincondition.getTimeOverCountdown().getTimeLeft())
Besitzer

Ich glaube, eine Deaktivierung mit Beginn der Enterphase ist derzeit nicht vorgesehen?

Ich glaube, eine Deaktivierung mit Beginn der Enterphase ist derzeit nicht vorgesehen?
Autor
Besitzer

Ist aber so auch im RelativePercent

Ist aber so auch im RelativePercent
Besitzer

Weil der RelativePercent von WS kommt.....

Weil der RelativePercent von WS kommt.....
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +129,4 @@
}
}
private static boolean validBlock(Block block) {
Besitzer

Methode bitte vor die Subklasse verschieben

Methode bitte vor die Subklasse verschieben
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +131,4 @@
private static boolean validBlock(Block block) {
Material material = block.getType();
if (ignoredBlocks.contains(material)) {
Besitzer

Es wird nur eine Auswahl an Blöcken erfasst, aber dann gibt es nochmal extra ignorierte Blöcke?!? Bitte was?

Es wäre evtl. am besten (und konfiurabelsten) die ignoredBlocks-Liste als die Liste der Blöcke, die gewertet wird, zu verwenden.

Es wird nur eine Auswahl an Blöcken erfasst, aber dann gibt es nochmal extra ignorierte Blöcke?!? Bitte was? Es wäre evtl. am besten (und konfiurabelsten) die ignoredBlocks-Liste als die Liste der Blöcke, die gewertet wird, zu verwenden.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +141,4 @@
|| blockData instanceof Rail) {
return true;
}
switch (material) {
Besitzer

Ich glaube, hier ist die Prüfung auf die BlockData und das Material redundant, für eine der beiden Varianten entscheiden.

Ich glaube, hier ist die Prüfung auf die BlockData und das Material redundant, für eine der beiden Varianten entscheiden.
Autor
Besitzer

Geht nicht ganz.

Geht nicht ganz.
Besitzer

Doch.

Doch.
YoyoNow markierte diese Unterhaltung als gelöst
YoyoNow hat 1 Commit 2021-05-13 12:52:31 +02:00 hinzugefügt
Update PercentWincondition
YoyoNow hat 1 Commit 2021-05-13 13:14:48 +02:00 hinzugefügt
Update PercentWincondition
YoyoNow hat 1 Commit 2021-05-13 13:20:11 +02:00 hinzugefügt
YoyoNow hat 1 Commit 2021-05-13 13:22:15 +02:00 hinzugefügt
YoyoNow hat 1 Commit 2021-05-13 13:23:03 +02:00 hinzugefügt
YoyoNow hat 1 Commit 2021-05-15 21:35:54 +02:00 hinzugefügt
Lixfel hat 2021-05-16 20:55:17 +02:00 Änderungen angefragt
@ -190,0 +220,4 @@
public Set<Characteristics> characteristics() {
return Collections.emptySet();
}
});
Besitzer

30+ Zeilen sind definitiv zu viel für so eine simple Sache. Bitte den Code kürzer/präziser machen.

30+ Zeilen sind definitiv zu viel für so eine simple Sache. Bitte den Code kürzer/präziser machen.
YoyoNow markierte diese Unterhaltung als gelöst
@ -26,0 +41,4 @@
return team.getPrefix() + "Schaden: " + (Math.round(100.0 * getPercent(team)) / 100.0) + "%";
}
abstract double getPercent(FightTeam team);
Besitzer

Jeder Prozentmodus braucht irgendwo beide Teams, die Map FightTeam -> TeamPercent müsste daher hierher abstrahierbar sein.

Aka: Die Teammap schon hier in PercentWincondition zur Verfügung stellen und füllen (parameter im Konstruktor übergeben lassen), getPercent gleich hier implementieren.

In der PointWincondition hast du dann einfach noch eine zweite Map

Jeder Prozentmodus braucht irgendwo beide Teams, die Map FightTeam -> TeamPercent müsste daher hierher abstrahierbar sein. Aka: Die Teammap schon hier in PercentWincondition zur Verfügung stellen und füllen (parameter im Konstruktor übergeben lassen), getPercent gleich hier implementieren. In der PointWincondition hast du dann einfach noch eine zweite Map
YoyoNow markierte diese Unterhaltung als gelöst
@ -26,0 +52,4 @@
public int totalBlocks = 0;
private int currentBlocks = 0;
private Consumer<TeamPercent> explodeConsumer;
Besitzer

Ich glaube, hier können ein paar Fields final gesetzt werden.

Ich glaube, hier können ein paar Fields final gesetzt werden.
YoyoNow markierte diese Unterhaltung als gelöst
@ -26,0 +62,4 @@
new OneShotStateDependent(wincondition, FightState.Running, () -> {
enableConsumer.accept(TeamPercent.this);
currentBlocks = totalBlocks;
}).register();
Besitzer

OneShotStateDependents werden schon im Konstruktor registered, bitte nicht dopplet registern.

OneShotStateDependents werden schon im Konstruktor registered, bitte nicht dopplet registern.
YoyoNow markierte diese Unterhaltung als gelöst
@ -34,3 +30,3 @@
import java.util.Map;
public class WinconditionPercentSystem extends Wincondition implements Listener, PrintableWincondition, PercentWincondition {
public class WinconditionPercentSystem extends PercentWincondition implements Listener {
Besitzer

Ist kein Listener mehr.

Ist kein Listener mehr.
YoyoNow markierte diese Unterhaltung als gelöst
@ -129,0 +132,4 @@
teamPercent.totalBlocks = currentBlocks.get();
}, teamPercent -> {
});
Besitzer

Zeilenumbrüche für eine leere Methode? teamPercent -> {}); feddich.

Zeilenumbrüche für eine leere Methode? teamPercent -> {}); feddich.
YoyoNow markierte diese Unterhaltung als gelöst
@ -41,4 +36,0 @@
Set<Material> ignored = new HashSet<>();
for(String s : Config.IgnoredBlocks)
ignored.add(Material.valueOf(s));
ignoredBlocks = Collections.unmodifiableSet(ignored);
Besitzer

WÜrde empfehlen, in der COnfig diesen Codeblock zu verwenden, 4 Zeilen sind da etwas kürzer :)

WÜrde empfehlen, in der COnfig diesen Codeblock zu verwenden, 4 Zeilen sind da etwas kürzer :)
YoyoNow markierte diese Unterhaltung als gelöst
@ -126,0 +56,4 @@
return new PercentWincondition.TeamPercent(fightTeam, Winconditions.PERCENT_SYSTEM, material -> {
return !Config.Blocks.contains(material);
}, teamPercent -> {
AtomicInteger currentBlocks = new AtomicInteger();
Besitzer

Muss kein AtomicInteger sein, wenn es sich um eine Objektvariable handelt

Muss kein AtomicInteger sein, wenn es sich um eine Objektvariable handelt
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +1,56 @@
package de.steamwar.fightsystem.winconditions;
Besitzer

Licence fehlt.

Licence fehlt.
YoyoNow markierte diese Unterhaltung als gelöst
YoyoNow hat 2 Commits 2021-05-22 15:39:54 +02:00 hinzugefügt
Lixfel hat 2021-05-23 09:26:54 +02:00 Änderungen angefragt
@ -26,0 +69,4 @@
currentBlocks = totalBlocks;
});
new StateDependentListener(wincondition, FightState.Running, this).register();
Besitzer

Hier am Ende des Konstruktors würde ich gleich dieses TeamPercent in die teamMap einfügen, dann musst du das nicht an den ganzen anderen Stellen machen (und kannst es nicht vergessen)

Hier am Ende des Konstruktors würde ich gleich dieses TeamPercent in die teamMap einfügen, dann musst du das nicht an den ganzen anderen Stellen machen (und kannst es nicht vergessen)
YoyoNow markierte diese Unterhaltung als gelöst
@ -56,3 +35,4 @@
if (Config.ActiveWinconditions.contains(Winconditions.PERCENT_SYSTEM)) {
printableWinconditions.add(this);
percentWincondition = this;
}
Besitzer

Dieser Codeblock sollte in die PercentWincondition verschoben werden (und dann aus den Subklassen entfernt werden). Points braucht ja nur ein @Override getDisplay

Dieser Codeblock sollte in die PercentWincondition verschoben werden (und dann aus den Subklassen entfernt werden). Points braucht ja nur ein @Override getDisplay
Autor
Besitzer

Kannst du mir das nochmal genauer erklären?

Kannst du mir das nochmal genauer erklären?
Besitzer

Diesen if-Block kannst du so in den Konstruktor der PercentWincondition verschieben.

Diesen if-Block kannst du so in den Konstruktor der PercentWincondition verschieben.
YoyoNow markierte diese Unterhaltung als gelöst
@ -129,0 +129,4 @@
currentBlocks.getAndIncrement();
}
});
teamPercent.totalBlocks = currentBlocks.get();
Besitzer

Der Codeblock zur Zählung ist in der RelativePercentWincondition "eleganter" gelöst. Evtl. diesen Codeblock in eine Funktion in PercentWincondition.TeamPercent auslagern, die dann hier aufgerufen wird? (Keine Codeduplication)

Der Codeblock zur Zählung ist in der RelativePercentWincondition "eleganter" gelöst. Evtl. diesen Codeblock in eine Funktion in PercentWincondition.TeamPercent auslagern, die dann hier aufgerufen wird? (Keine Codeduplication)
YoyoNow markierte diese Unterhaltung als gelöst
@ -134,2 +137,2 @@
int ownBlocks = percent.getBlockCount();
int enemyBlocks = teamMap.get(Fight.getOpposite(team)).percent.getBlockCount();
int ownBlocks = percent.totalBlocks;
int enemyBlocks = teamMap.get(Fight.getOpposite(team)).percent.totalBlocks;
Besitzer

Wie stellst du sicher, dass beim Aufruf dieser Enable-Methode die Enable-Methode des TeamPercents schon aufgerufen wurde, d.h. die Blöcke bereits gezählt wurden?

Wie stellst du sicher, dass beim Aufruf dieser Enable-Methode die Enable-Methode des TeamPercents schon aufgerufen wurde, d.h. die Blöcke bereits gezählt wurden?
Autor
Besitzer

Keine Ahnung Ideen?

Keine Ahnung Ideen?
Besitzer

Nein, das machst du nicht, daher wird das so nicht funktionieren (aka. verbuggt).

Idee 1: Du rufst vorher die enable des TeamPercents auf.

Idee 2: (Sauberer) Du übergibst im Konstrutor des TeamPercents eine Methode, welche am Ende der enable von TeamPercent aufgerufen wird...

Nein, das machst du nicht, daher wird das so nicht funktionieren (aka. verbuggt). Idee 1: Du rufst vorher die enable des TeamPercents auf. Idee 2: (Sauberer) Du übergibst im Konstrutor des TeamPercents eine Methode, welche am Ende der enable von TeamPercent aufgerufen wird...
YoyoNow markierte diese Unterhaltung als gelöst
@ -117,4 +44,0 @@
private int currentBlocks(){
// Entern active
if(!Config.EnterStages.isEmpty() && Config.EnterStages.get(0) >= Wincondition.getTimeOverCountdown().getTimeLeft())
return currentBlocks;
Besitzer

Hier wird ein wichtiges Feature für die RelativePercentWincondition und die PointsWincondition enfernt: Wenn die Enterphase begonnen hat, dürfen bei diesen beiden Winconditions keine Prozente mehr gezählt werden.

Hier wird ein wichtiges Feature für die RelativePercentWincondition und die PointsWincondition enfernt: Wenn die Enterphase begonnen hat, dürfen bei diesen beiden Winconditions keine Prozente mehr gezählt werden.
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +43,4 @@
}
private PercentWincondition.TeamPercent create(FightTeam fightTeam) {
return new PercentWincondition.TeamPercent(fightTeam, Winconditions.PERCENT_SYSTEM, material -> {
Besitzer

Falsche Wincondition

Falsche Wincondition
YoyoNow markierte diese Unterhaltung als gelöst
@ -0,0 +50,4 @@
if (Config.Blocks.contains(world.getBlockAt(x, y, z).getType())) {
teamPercent.totalBlocks++;
}
});
Besitzer

Codedup.

Codedup.
YoyoNow markierte diese Unterhaltung als gelöst
YoyoNow hat 1 Commit 2021-05-23 17:00:01 +02:00 hinzugefügt
YoyoNow hat 1 Commit 2021-05-23 19:23:58 +02:00 hinzugefügt
Update WinconditionPoints
Lixfel hat 1 Commit 2021-05-24 08:24:36 +02:00 hinzugefügt
Signed-off-by: Lixfel <agga-games@gmx.de>
Lixfel hat 1 Commit 2021-05-24 08:30:05 +02:00 hinzugefügt
Signed-off-by: Lixfel <agga-games@gmx.de>
Besitzer

@YoyoNow nachdem ich da auch nochmal rumgepfuscht habe, könntest du nochmal ein Review durchführen?

@YoyoNow nachdem ich da auch nochmal rumgepfuscht habe, könntest du nochmal ein Review durchführen?
YoyoNow hat 1 Commit 2021-06-06 14:57:56 +02:00 hinzugefügt
Autor
Besitzer

Ich kann kein Review durchführen, da der PR von mir ist.

Ich kann kein Review durchführen, da der PR von mir ist.
Autor
Besitzer

Ich glaube aber bis auf public static final World world... habe ich nichts gefunden.

Ich glaube aber bis auf `public static final World world...` habe ich nichts gefunden.
YoyoNow hat 1 Commit 2021-07-12 09:42:16 +02:00 hinzugefügt
Autor
Besitzer

Wie steht es um diesen PR?

Wie steht es um diesen PR?
Besitzer

Müssen glaube mal alle %Winconditions mal durchgetestet werden, ansonsten sollte der nach meinem Umbau passen

Müssen glaube mal alle %Winconditions mal durchgetestet werden, ansonsten sollte der nach meinem Umbau passen
Autor
Besitzer

Ok, dann mache ich dies mal bei zeiten oder gebe es einen anderem Dev

Ok, dann mache ich dies mal bei zeiten oder gebe es einen anderem Dev
Lixfel hat sich das Issue 2021-09-27 16:56:53 +02:00 selbst zugewiesen
Lixfel hat 1 Commit 2021-09-27 17:25:12 +02:00 hinzugefügt
Merge branch 'master' into RedstoneWincondition
Einige Prüfungen sind fehlgeschlagen
SteamWarCI Build failed
ce9127cacf
# Conflicts:
#	FightSystem_Core/src/de/steamwar/fightsystem/winconditions/WinconditionPercentSystem.java
#	FightSystem_Core/src/de/steamwar/fightsystem/winconditions/WinconditionRelativePercent.java
Lixfel hat diesen Pull-Request 2021-10-29 12:00:02 +02:00 geschlossen
Lixfel löschte die Branch RedstoneWincondition 2021-10-29 12:00:09 +02:00
Dieses Repo ist archiviert. Du kannst Pull-Requests nicht kommentieren.
Keine Beschreibung angegeben.