SteamWar/FightSystem
Archiviert
13
1

Arrows Stopping in Techhider Blocks #208

Manuell gemergt
Lixfel hat 17 Commits von arrow-in-techhider nach master 2021-01-02 09:20:42 +01:00 zusammengeführt
3 geänderte Dateien mit 38 neuen und 22 gelöschten Zeilen
Nur Änderungen aus Commit 030bfbc6a1 werden angezeigt - Alle Commits anzeigen

Datei anzeigen

@ -20,12 +20,12 @@
package de.steamwar.fightsystem.utils;
import org.bukkit.block.Block;
import org.bukkit.craftbukkit.v1_15_R1.block.CraftBlock;
import org.bukkit.craftbukkit.v1_14_R1.block.CraftBlock;
public class ArrowStopper_15 {
private ArrowStopper_15(){}
public class ArrowStopper_14 {
private ArrowStopper_14(){}
static int blockToId(Block block){
return net.minecraft.server.v1_15_R1.Block.REGISTRY_ID.getId(((CraftBlock)block).getNMS());
return net.minecraft.server.v1_14_R1.Block.REGISTRY_ID.getId(((CraftBlock)block).getNMS());
Veraltet
Review

Funktioniert nur für die 1.14, für die 1.15 ist eine separate Klasse erforderlich.

Funktioniert nur für die 1.14, für die 1.15 ist eine separate Klasse erforderlich.
}
}

Datei anzeigen

@ -21,8 +21,8 @@ package de.steamwar.fightsystem.utils;
import org.bukkit.block.Block;
public class ArrowStopper_12 {
private ArrowStopper_12(){}
public class ArrowStopper_8 {
private ArrowStopper_8(){}
@SuppressWarnings("deprecation")
static int blockToId(Block block){

Datei anzeigen

@ -31,17 +31,22 @@ import org.bukkit.block.BlockFace;
import org.bukkit.entity.AbstractArrow;
import org.bukkit.entity.Entity;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.HandlerList;
import org.bukkit.event.Listener;
import org.bukkit.event.entity.EntityShootBowEvent;
import org.bukkit.event.player.PlayerPickupArrowEvent;
import org.bukkit.scheduler.BukkitTask;
import java.util.*;
public class ArrowStopper implements StateDependent {
public class ArrowStopper implements StateDependent, Listener {
Veraltet
Review

Wenn das ganze StateDependent und Listener ist, wäre es evtl. eleganter, das ganze als listener zu haben, und für den Task dann einfach die enable und disable-Methode überschreiben und die super()-Methode mit aufzurufen.

Wenn das ganze StateDependent und Listener ist, wäre es evtl. eleganter, das ganze als listener zu haben, und für den Task dann einfach die enable und disable-Methode überschreiben und die super()-Methode mit aufzurufen.
private BukkitTask task;
private static final HashMap<Entity, Location> LAST_LOCATION = new HashMap<>();
public static void init() {
if(Config.ArrowTechhiderCollision == -1)
if(Config.ArrowTechhiderCollision == 0)
return;
FightSystem.registerStateDependent(new ArrowStopper());
}
@ -50,21 +55,29 @@ public class ArrowStopper implements StateDependent {
Collection<Entity> arrows = Bukkit.getWorlds().get(0).getEntitiesByClasses(AbstractArrow.class);
Veraltet
Review

Landen arrows wirklich in Blöcken?

Landen arrows wirklich in Blöcken?
if(arrows.isEmpty())
Veraltet
Review

Auch wenn die arrows Empty sind, sollten die Pfeile aus der LAST_LOCATION entfernt werden (also mehr oder weniger der normale Programmfluss genommen werden), oder?

Auch wenn die arrows Empty sind, sollten die Pfeile aus der LAST_LOCATION entfernt werden (also mehr oder weniger der normale Programmfluss genommen werden), oder?
return;
for (Entity entity : arrows) {
if(entity.getTicksLived() > Config.ArrowTechhiderCollision ||
((AbstractArrow) entity).isInBlock()){
LAST_LOCATION.remove(entity);
continue;
for (Map.Entry<Entity, Location> e : LAST_LOCATION.entrySet()) {
Veraltet
Review

Hätte es gerne, dass du zur kognitiven Komplexitätsreduktion einfach am Ende prüfst (beim derzeitigen else), wie lange die Entity schon gelebt hat, und sie dann ggf. einfach nicht mehr einfügst.

Hätte es gerne, dass du zur kognitiven Komplexitätsreduktion einfach am Ende prüfst (beim derzeitigen else), wie lange die Entity schon gelebt hat, und sie dann ggf. einfach nicht mehr einfügst.
Veraltet
Review

Bitte auch noch "tote" Arrows/ stillstehende Arrows rauswerfen.

Bitte auch noch "tote" Arrows/ stillstehende Arrows rauswerfen.
if(checkBlocks(e.getKey().getLocation().getBlock(), e.getValue().getBlock()))
Veraltet
Review

Wenn man so häufig e.getKey() verwendet (weiter unten ja auch noch), ist es einfach wesentlich besser lesbar, wenn man das dann einfach mal als temporäre extravariable deklariert.

Wenn man so häufig e.getKey() verwendet (weiter unten ja auch noch), ist es einfach wesentlich besser lesbar, wenn man das dann einfach mal als temporäre extravariable deklariert.
e.getKey().remove();
Veraltet
Review

Das dürfte glaube ich nicht den Wert aus dem Set entfernen, sondern nur die Entity aus der Welt. LAST_LOCATION wird damit immer weiter zugemüllt.

Das dürfte glaube ich nicht den Wert aus dem Set entfernen, sondern nur die Entity aus der Welt. LAST_LOCATION wird damit immer weiter zugemüllt.
else {
Veraltet
Review

Bitte reduziere mal deine Map-Operationen. Optimierungsidee: Du machst LAST_LOCATION.remove(), und wenn du null bekommst, nimmst du deine EyeLocation, 1,5 Mapoperationen weniger.

Bitte reduziere mal deine Map-Operationen. Optimierungsidee: Du machst LAST_LOCATION.remove(), und wenn du null bekommst, nimmst du deine EyeLocation, 1,5 Mapoperationen weniger.
if(e.getKey().getTicksLived() <= Config.ArrowTechhiderCollision ||
!((AbstractArrow) e.getKey()).isInBlock()) {
LAST_LOCATION.put(e.getKey(), e.getKey().getLocation());
Veraltet
Review

Ich glaube, replace() wäre hier die angebrachtere Operation

Ich glaube, replace() wäre hier die angebrachtere Operation
}
if(!LAST_LOCATION.containsKey(entity))
LAST_LOCATION.put(entity, ((Player) ((AbstractArrow) entity).getShooter()).getEyeLocation());
Location last = LAST_LOCATION.remove(entity);
if(checkBlocks(entity.getLocation().getBlock(), last.getBlock()))
entity.remove();
else
LAST_LOCATION.put(entity, entity.getLocation());
}
}
}
@EventHandler()
Veraltet
Review

Wozu die Klammern?

Wozu die Klammern?
public void onEntityShootBow(EntityShootBowEvent event) {
if(!(event.getEntity() instanceof Player))
Veraltet
Review

Da eh kaum etwas anderes als Spieler den Pfeil schießen werden, ist die Filterung hier eigentlich unnötig

Da eh kaum etwas anderes als Spieler den Pfeil schießen werden, ist die Filterung hier eigentlich unnötig
return;
LAST_LOCATION.put(event.getProjectile(), event.getEntity().getEyeLocation());
Veraltet
Review

Eine solche Funktionalität sollte bereits mit dem Techhider bzw. RecordSystem enthalten sein, wenn würde ich diese generalisieren.

Eine solche Funktionalität sollte bereits mit dem Techhider bzw. RecordSystem enthalten sein, wenn würde ich diese generalisieren.
}
@EventHandler
public void onPlayerPickupArrow(PlayerPickupArrowEvent event) {
Veraltet
Review

Spieler heben keine Pfeile auf (ist unterbunden)

Spieler heben keine Pfeile auf (ist unterbunden)
LAST_LOCATION.remove(event.getArrow());
}
@Override
public Set<FightState> enabled() {
@ -73,6 +86,7 @@ public class ArrowStopper implements StateDependent {
@Override
public void enable() {
Bukkit.getPluginManager().registerEvents(this, FightSystem.getPlugin());
task = Bukkit.getScheduler().runTaskTimerAsynchronously(FightSystem.getPlugin(), this::run, 1, 1);
}
@ -80,6 +94,7 @@ public class ArrowStopper implements StateDependent {
public void disable() {
task.cancel();
LAST_LOCATION.clear();
HandlerList.unregisterAll(this);
}
private static int blockToId(Block block){
@ -88,10 +103,11 @@ public class ArrowStopper implements StateDependent {
case 9:
case 10:
case 12:
return ArrowStopper_12.blockToId(block);
return ArrowStopper_8.blockToId(block);
case 14:
case 15:
default:
return ArrowStopper_15.blockToId(block);
return ArrowStopper_14.blockToId(block);
}
}