RedstoneWincondition #274
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/FightSystem#274
Laden…
In neuem Issue referenzieren
Einen Benutzer sperren
Keine Beschreibung angegeben.
Branch "RedstoneWincondition" 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?
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) {
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.
@ -0,0 +32,4 @@
Set<Material> ignored = new HashSet<>();
for (String s : Config.IgnoredBlocks)
ignored.add(Material.valueOf(s));
ignoredBlocks = Collections.unmodifiableSet(ignored);
Richt mir ein bisschen nach Code-Duplication, bitte auslagern, dass der Code nur einmal da ist.
@ -0,0 +45,4 @@
if (Config.ActiveWinconditions.contains(Winconditions.RELATIVE_REDSTONE_PERCENT)) {
printableWinconditions.add(this);
percentWincondition = this;
Bukkit.getPluginManager().registerEvents(this, FightSystem.getPlugin());
Bitte einen StateDependentListener verwenden.
Kann ich aber nicht hier in der Klasse machen, weil 2 mal Extend wird wohl nichts!
@ -0,0 +56,4 @@
@Override
public String getDisplay(FightTeam team) {
return team.getPrefix() + "Schaden: " + (Math.round(100.0 * getPercent(team)) / 100.0) + "%";
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)
@ -0,0 +60,4 @@
}
@EventHandler
public void onBlockPlace(BlockPlaceEvent event) {
Den Listener kannst du weglassen, was möchtest du damit überhaupt? Es sind eh nur die zyklischen Prüfungen relevant.
Nein, wenn ein Block plaziert wurde, darf man keine Prozente zurückbekommen.
Dann wäre allerdings der explosionsbasierte Messansatz besser als der zeitbasiert prüfende.
@ -0,0 +70,4 @@
}
}
public static class TeamPercent extends StateDependent {
Der Code hier liest sich wie Copy+Paste aus der RelativeWincondition. Codeduplication bitte entfernen (aka generalisieren)
@ -0,0 +81,4 @@
super(Winconditions.RELATIVE_REDSTONE_PERCENT, FightState.Running);
this.team = team;
this.currentBlocks = 1;
register();
Fände es nicer, wenn hier ein StateDependentTask und ein OneShotStateDependent statt des extends StateDependent verwendet wird.
@ -0,0 +115,4 @@
private int currentBlocks() {
// Entern active
if (!Config.EnterStages.isEmpty() && Config.EnterStages.get(0) >= Wincondition.getTimeOverCountdown().getTimeLeft())
Ich glaube, eine Deaktivierung mit Beginn der Enterphase ist derzeit nicht vorgesehen?
Ist aber so auch im RelativePercent
Weil der RelativePercent von WS kommt.....
@ -0,0 +129,4 @@
}
}
private static boolean validBlock(Block block) {
Methode bitte vor die Subklasse verschieben
@ -0,0 +131,4 @@
private static boolean validBlock(Block block) {
Material material = block.getType();
if (ignoredBlocks.contains(material)) {
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.
@ -0,0 +141,4 @@
|| blockData instanceof Rail) {
return true;
}
switch (material) {
Ich glaube, hier ist die Prüfung auf die BlockData und das Material redundant, für eine der beiden Varianten entscheiden.
Geht nicht ganz.
Doch.
@ -190,0 +220,4 @@
public Set<Characteristics> characteristics() {
return Collections.emptySet();
}
});
30+ Zeilen sind definitiv zu viel für so eine simple Sache. Bitte den Code kürzer/präziser machen.
@ -26,0 +41,4 @@
return team.getPrefix() + "Schaden: " + (Math.round(100.0 * getPercent(team)) / 100.0) + "%";
}
abstract double getPercent(FightTeam team);
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
@ -26,0 +52,4 @@
public int totalBlocks = 0;
private int currentBlocks = 0;
private Consumer<TeamPercent> explodeConsumer;
Ich glaube, hier können ein paar Fields final gesetzt werden.
@ -26,0 +62,4 @@
new OneShotStateDependent(wincondition, FightState.Running, () -> {
enableConsumer.accept(TeamPercent.this);
currentBlocks = totalBlocks;
}).register();
OneShotStateDependents werden schon im Konstruktor registered, bitte nicht dopplet registern.
@ -34,3 +30,3 @@
import java.util.Map;
public class WinconditionPercentSystem extends Wincondition implements Listener, PrintableWincondition, PercentWincondition {
public class WinconditionPercentSystem extends PercentWincondition implements Listener {
Ist kein Listener mehr.
@ -129,0 +132,4 @@
teamPercent.totalBlocks = currentBlocks.get();
}, teamPercent -> {
});
Zeilenumbrüche für eine leere Methode? teamPercent -> {}); feddich.
@ -41,4 +36,0 @@
Set<Material> ignored = new HashSet<>();
for(String s : Config.IgnoredBlocks)
ignored.add(Material.valueOf(s));
ignoredBlocks = Collections.unmodifiableSet(ignored);
WÜrde empfehlen, in der COnfig diesen Codeblock zu verwenden, 4 Zeilen sind da etwas kürzer :)
@ -126,0 +56,4 @@
return new PercentWincondition.TeamPercent(fightTeam, Winconditions.PERCENT_SYSTEM, material -> {
return !Config.Blocks.contains(material);
}, teamPercent -> {
AtomicInteger currentBlocks = new AtomicInteger();
Muss kein AtomicInteger sein, wenn es sich um eine Objektvariable handelt
@ -0,0 +1,56 @@
package de.steamwar.fightsystem.winconditions;
Licence fehlt.
@ -26,0 +69,4 @@
currentBlocks = totalBlocks;
});
new StateDependentListener(wincondition, FightState.Running, this).register();
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)
@ -56,3 +35,4 @@
if (Config.ActiveWinconditions.contains(Winconditions.PERCENT_SYSTEM)) {
printableWinconditions.add(this);
percentWincondition = this;
}
Dieser Codeblock sollte in die PercentWincondition verschoben werden (und dann aus den Subklassen entfernt werden). Points braucht ja nur ein @Override getDisplay
Kannst du mir das nochmal genauer erklären?
Diesen if-Block kannst du so in den Konstruktor der PercentWincondition verschieben.
@ -129,0 +129,4 @@
currentBlocks.getAndIncrement();
}
});
teamPercent.totalBlocks = currentBlocks.get();
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)
@ -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;
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?
Keine Ahnung Ideen?
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...
@ -117,4 +44,0 @@
private int currentBlocks(){
// Entern active
if(!Config.EnterStages.isEmpty() && Config.EnterStages.get(0) >= Wincondition.getTimeOverCountdown().getTimeLeft())
return currentBlocks;
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.
@ -0,0 +43,4 @@
}
private PercentWincondition.TeamPercent create(FightTeam fightTeam) {
return new PercentWincondition.TeamPercent(fightTeam, Winconditions.PERCENT_SYSTEM, material -> {
Falsche Wincondition
@ -0,0 +50,4 @@
if (Config.Blocks.contains(world.getBlockAt(x, y, z).getType())) {
teamPercent.totalBlocks++;
}
});
Codedup.
@YoyoNow nachdem ich da auch nochmal rumgepfuscht habe, könntest du nochmal ein Review durchführen?
Ich kann kein Review durchführen, da der PR von mir ist.
Ich glaube aber bis auf
public static final World world...
habe ich nichts gefunden.Wie steht es um diesen PR?
Müssen glaube mal alle %Winconditions mal durchgetestet werden, ansonsten sollte der nach meinem Umbau passen
Ok, dann mache ich dies mal bei zeiten oder gebe es einen anderem Dev