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
Nur Änderungen aus Commit 25190098d6 werden angezeigt - Alle Commits anzeigen

Datei anzeigen

@ -1,3 +1,22 @@
/*
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

License fehlt.

License fehlt.
This file is a part of the SteamWar software.
Copyright (C) 2020 SteamWar.de-Serverteam
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package de.steamwar.fightsystem.event;
import de.steamwar.fightsystem.Config;
@ -17,6 +36,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.Random;
import java.util.concurrent.atomic.AtomicInteger;
import org.bukkit.scheduler.BukkitTask;
public class HellsBells {
@ -66,59 +86,21 @@ public class HellsBells {
public HellsBells() {
new StateDependent(Winconditions.HELLS_BELLS, FightState.Running) {
private HellsBellsEvent event = null;
private final World world = Bukkit.getWorlds().get(0);
private final int xLength = Config.RedPasteRegion.getMaxX() - Config.RedPasteRegion.getMinX();
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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.
private final int zLength = Config.RedPasteRegion.getMaxZ() - Config.RedPasteRegion.getMinZ();
private State current = State.PRE;
private int currentDrops = 0;
private HellsBellsCountdown currentCountdown;
private BukkitTask currentDropping;
private final List<String> startMessages = Arrays.asList("§c!!Achtung!! Bomber im Anflug, noch ca. eine Minute bis zur Ankunft.",
Zeanon markierte diese Unterhaltung als gelöst
Review

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)
"§cBomber im Anflug, ca. eine Minute bis zur Ankunft.",
"§cBomber auf dem Radar gesichtet, geschätzte Ankunftszeit: eine Minute.",
"§cUnbekanntes Flugobjekt gesichtet, trifft in ca. einer Minute ein.",
"§cFlugobjekt gesichtet. Ankunft in ca. einer Minute.",
"§cBomber erschienen, Ankunft ca. eine Minute.");
@Override
public void enable() {
if (event != null) {
event.terminate();
}
event = new HellsBellsEvent();
event.init();
}
@Override
public void disable() {
if (event != null) {
event.terminate();
}
event = null;
}
}.register();
}
private static class HellsBellsEvent {
private final World world = Bukkit.getWorlds().get(0);
private final int xLength = Config.RedPasteRegion.getMaxX() - Config.RedPasteRegion.getMinX();
private final int zLength = Config.RedPasteRegion.getMaxZ() - Config.RedPasteRegion.getMinZ();
private State current = State.NONE;
private int currentDrops = 0;
private HellsBellsCountdown currentCountdown;
private static final List<String> startMessages = Arrays.asList("§c!!Achtung!! Bomber im Anflug, noch ca. eine Minute bis zur Ankunft.",
"§cBomber im Anflug, ca. eine Minute bis zur Ankunft.",
"§cBomber auf dem Radar gesichtet, geschätzte Ankunftszeit: eine Minute.",
"§cUnbekanntes Flugobjekt gesichtet, trifft in ca. einer Minute ein.",
"§cFlugobjekt gesichtet. Ankunft in ca. einer Minute.",
"§cBomber erschienen, Ankunft ca. eine Minute.");
public void init() {
current = State.PRE;
currentDrops = 0;
currentCountdown = null;
startCountdown();
}
public void terminate() {
current = State.NONE;
if (currentCountdown != null) {
currentCountdown.disable();
}
}
public void startCountdown() {
if (current != State.NONE) {
public void startCountdown() {
if (current == State.PRE) {
Bukkit.broadcastMessage(FightSystem.PREFIX + (startMessages.get(random.nextInt(startMessages.size()))));
current = current.getNext();
@ -131,31 +113,28 @@ public class HellsBells {
currentCountdown = new HellsBellsCountdown(current.MIN_TIME + random.nextInt(current.MAX_TIME - current.MIN_TIME), SWSound.BLOCK_NOTE_BASS, true);
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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...
currentCountdown.enable();
Zeanon markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Wird der State NONE wirklich benötigt?

Wird der State NONE wirklich benötigt?
}
}
public void drop() {
Direction direction = Direction.getRandom();
public void drop() {
Direction direction = Direction.getRandom();
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());
int yOffset = getHeightStart();
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());
int yOffset = getHeightStart();
Point redStart;
Point blueStart;
Point redStart;
Point blueStart;
if (direction == Direction.NORTH || direction == Direction.WEST) {
redStart = new Point(Config.RedPasteRegion.getMaxX() - xOffset, Config.RedExtendRegion.getMaxY() + yOffset, Config.RedPasteRegion.getMaxZ() - zOffset);
blueStart = new Point(Config.BluePasteRegion.getMinX() + xOffset, Config.BlueExtendRegion.getMaxY() + yOffset, Config.BluePasteRegion.getMinZ() + zOffset);
} else {
redStart = new Point(Config.RedPasteRegion.getMinX() + xOffset, Config.RedExtendRegion.getMaxY() + yOffset, Config.RedPasteRegion.getMinZ() + zOffset);
blueStart = new Point(Config.BluePasteRegion.getMaxX() - xOffset, Config.BlueExtendRegion.getMaxY() + yOffset, Config.BluePasteRegion.getMaxZ() - zOffset);
}
if (direction == Direction.NORTH || direction == Direction.WEST) {
redStart = new Point(Config.RedPasteRegion.getMaxX() - xOffset, Config.RedExtendRegion.getMaxY() + yOffset, Config.RedPasteRegion.getMaxZ() - zOffset);
blueStart = new Point(Config.BluePasteRegion.getMinX() + xOffset, Config.BlueExtendRegion.getMaxY() + yOffset, Config.BluePasteRegion.getMinZ() + zOffset);
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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

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

Nein leider bietet die normale Random implementation dies nicht an.

Nein leider bietet die normale Random implementation dies nicht an.
} else {
redStart = new Point(Config.RedPasteRegion.getMinX() + xOffset, Config.RedExtendRegion.getMaxY() + yOffset, Config.RedPasteRegion.getMinZ() + zOffset);
blueStart = new Point(Config.BluePasteRegion.getMaxX() - xOffset, Config.BlueExtendRegion.getMaxY() + yOffset, Config.BluePasteRegion.getMaxZ() - zOffset);
}
new BukkitRunnable() {
@Override
public void run() {
currentDropping = Bukkit.getScheduler().runTaskTimer(FightSystem.getPlugin(), () -> {
for (int w = 0; w < width; w++) {
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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 ?:.
if (direction == Direction.NORTH || direction == Direction.WEST) {
world.spawnEntity(redStart.subtractAndToLocation(world, direction.dx * length.get() + w * direction.other().dx, 0, direction.dz * length.get() + w * direction.other().dz), EntityType.PRIMED_TNT);
@ -167,104 +146,113 @@ public class HellsBells {
world.spawnEntity(blueStart.subtractAndToLocation(world, direction.dx * length.get() + w * direction.other().dx, 0, direction.dz * length.get() + w * direction.other().dz), EntityType.PRIMED_TNT);
}
}
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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

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

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.
if (length.addAndGet(-2) <= 0) {
cancel();
currentDropping.cancel();
}
}, 0L, 4L);
}
private int getLengthStart(int regionSize, int length) {
return random.nextInt(regionSize - length);
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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

Üblicherweise verwenden wir eher Bukkit...getScheduler().runTaskTimer(Plugin, lambda-Methode, Zeit(en))
}
private int getWidthStart(int regionSize, int length) {
return random.nextInt(regionSize - length);
}
private int getHeightStart() {
return 5 + random.nextInt(15);
}
class HellsBellsCountdown extends Countdown {
public HellsBellsCountdown(int time, SWSound sound, boolean level) {
super(time, sound, level);
}
}.runTaskTimer(FightSystem.getPlugin(), 0, 4L);
}
private int getLengthStart(int regionSize, int length) {
return random.nextInt(regionSize - length);
}
@Override
public String countdownCounting() {
return "bis die Bomben fallen";
}
private int getWidthStart(int regionSize, int length) {
return random.nextInt(regionSize - length);
}
@Override
public void countdownFinished() {
drop();
private int getHeightStart() {
return 5 + random.nextInt(15);
}
private class HellsBellsCountdown extends Countdown {
public HellsBellsCountdown(int time, SWSound sound, boolean level) {
super(time, sound, level);
startCountdown();
}
}
@Override
public String countdownCounting() {
return "bis die Bomben fallen";
}
@Override
public void countdownFinished() {
drop();
public void enable() {
startCountdown();
}
@Override
public void disable() {
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

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...
currentCountdown.disable();
}
}.register();
}
private static class Point {
private final int x;
private final int y;
private final int z;
public Point(int x, int y, int z) {
this.x = x;
this.y = y;
this.z = z;
Zeanon markierte diese Unterhaltung als gelöst
Review

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.
}
YoyoNow markierte diese Unterhaltung als gelöst Veraltet
Veraltet
Review

Warum geht es nicht, Location oder Vector zu verwenden?

Warum geht es nicht, Location oder Vector zu verwenden?
Veraltet
Review

Weil Location und Vektor zu mehr undurchsichtigem code führt

Weil Location und Vektor zu mehr undurchsichtigem code führt
private static class Point {
private final int x;
private final int y;
private final int z;
public Point(int x, int y, int z) {
this.x = x;
this.y = y;
this.z = z;
}
public Location addAndToLocation(World world, int x, int y, int z) {
return new Location(world, this.x + x, this.y + y, this.z + z); //NOSONAR
}
public Location subtractAndToLocation(World world, int x, int y, int z) {
return new Location(world, this.x - x, this.y - y, this.z - z); //NOSONAR
}
public Location addAndToLocation(World world, int x, int y, int z) {
return new Location(world, this.x + x, this.y + y, this.z + z); //NOSONAR
}
private enum State {
public Location subtractAndToLocation(World world, int x, int y, int z) {
return new Location(world, this.x - x, this.y - y, this.z - z); //NOSONAR
}
}
NONE(0, 0, 0),
PRE(60, 80, 1),
FIRST(40, 60, 5),
SECOND(30, 40, 6),
THIRD(20, 30, 6),
FOURTH(10, 20, 7),
LAST(5, 10, 0);
private enum State {
NONE(0, 0, 0),
PRE(60, 80, 1),
FIRST(40, 60, 5),
SECOND(30, 40, 6),
THIRD(20, 30, 6),
FOURTH(10, 20, 7),
LAST(5, 10, 0);
State(int minTime, int maxTime, int switchAfter) {
this.MIN_TIME = minTime;
this.MAX_TIME = maxTime;
this.SWITCH_AFTER = switchAfter;
}
State(int minTime, int maxTime, int switchAfter) {
this.MIN_TIME = minTime;
this.MAX_TIME = maxTime;
this.SWITCH_AFTER = switchAfter;
}
private final int MIN_TIME; //NOSONAR
private final int MAX_TIME; //NOSONAR
private final int SWITCH_AFTER; //NOSONAR
private final int MIN_TIME; //NOSONAR
private final int MAX_TIME; //NOSONAR
private final int SWITCH_AFTER; //NOSONAR
public State getNext() {
switch (this) {
case PRE:
return FIRST;
case FIRST:
return SECOND;
case SECOND:
return THIRD;
case THIRD:
return FOURTH;
case FOURTH:
case LAST:
return LAST;
default:
return NONE;
}
public State getNext() {
switch (this) {
case PRE:
return FIRST;
case FIRST:
return SECOND;
case SECOND:
return THIRD;
case THIRD:
return FOURTH;
case FOURTH:
case LAST:
return LAST;
default:
return NONE;
}
}
}