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
Besitzer

Closes: #99

Closes: #99
Lixfel hat 2020-11-27 17:10:27 +01:00 Änderungen angefragt
@ -0,0 +26,4 @@
@SuppressWarnings("deprecation")
static int blockToId(Block block){
return block.getTypeId() << 4 + block.getData();
Besitzer

In der 1.12 ist in Config.HiddenBlocks nur block.getTypeId() ohne << 4 + getData()

In der 1.12 ist in Config.HiddenBlocks nur block.getTypeId() ohne << 4 + getData()
@ -0,0 +71,4 @@
task.cancel();
}
private static int blockToId(Block block){
Besitzer

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.
Lixfel hat 2020-11-30 17:21:30 +01:00 Änderungen angefragt
@ -0,0 +21,4 @@
import org.bukkit.block.Block;
public class ArrowStopper_12 {
Besitzer

Das müsste ArrowStopper_8 sein (kleinste unterstützte Version)

Das müsste ArrowStopper_8 sein (kleinste unterstützte Version)
@ -0,0 +22,4 @@
import org.bukkit.block.Block;
import org.bukkit.craftbukkit.v1_15_R1.block.CraftBlock;
public class ArrowStopper_15 {
Besitzer

Das müsste ArrowStopper_14 sein (kleinste unterstützte Version)

Das müsste ArrowStopper_14 sein (kleinste unterstützte Version)
@ -0,0 +52,4 @@
return;
for (Entity entity : arrows) {
if(entity.getTicksLived() > Config.ArrowTechhiderCollision ||
((AbstractArrow) entity).isInBlock()){
Besitzer

Landen arrows wirklich in Blöcken?

Landen arrows wirklich in Blöcken?
@ -0,0 +55,4 @@
((AbstractArrow) entity).isInBlock()){
LAST_LOCATION.remove(entity);
continue;
}
Besitzer

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.
@ -0,0 +58,4 @@
}
if(!LAST_LOCATION.containsKey(entity))
LAST_LOCATION.put(entity, ((Player) ((AbstractArrow) entity).getShooter()).getEyeLocation());
Location last = LAST_LOCATION.remove(entity);
Besitzer

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.
Lixfel hat 2020-12-07 21:22:52 +01:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Ist das schon als Production-Ready angesehen oder noch als WIP angedacht?

Ist das schon als Production-Ready angesehen oder noch als WIP angedacht?
@ -0,0 +26,4 @@
private ArrowStopper_14(){}
static int blockToId(Block block){
return net.minecraft.server.v1_14_R1.Block.REGISTRY_ID.getId(((CraftBlock)block).getNMS());
Besitzer

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.
@ -0,0 +40,4 @@
import java.util.*;
public class ArrowStopper implements StateDependent, Listener {
Besitzer

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.
@ -0,0 +53,4 @@
private void run() {
Collection<Entity> arrows = Bukkit.getWorlds().get(0).getEntitiesByClasses(AbstractArrow.class);
if(arrows.isEmpty())
Besitzer

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?
@ -0,0 +55,4 @@
Collection<Entity> arrows = Bukkit.getWorlds().get(0).getEntitiesByClasses(AbstractArrow.class);
if(arrows.isEmpty())
return;
for (Map.Entry<Entity, Location> e : LAST_LOCATION.entrySet()) {
Besitzer

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

Bitte auch noch "tote" Arrows/ stillstehende Arrows rauswerfen.
@ -0,0 +56,4 @@
if(arrows.isEmpty())
return;
for (Map.Entry<Entity, Location> e : LAST_LOCATION.entrySet()) {
if(checkBlocks(e.getKey().getLocation().getBlock(), e.getValue().getBlock()))
Besitzer

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.
@ -0,0 +57,4 @@
return;
for (Map.Entry<Entity, Location> e : LAST_LOCATION.entrySet()) {
if(checkBlocks(e.getKey().getLocation().getBlock(), e.getValue().getBlock()))
e.getKey().remove();
Besitzer

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.
@ -0,0 +61,4 @@
else {
if(e.getKey().getTicksLived() <= Config.ArrowTechhiderCollision ||
!((AbstractArrow) e.getKey()).isInBlock()) {
LAST_LOCATION.put(e.getKey(), e.getKey().getLocation());
Besitzer

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

Ich glaube, replace() wäre hier die angebrachtere Operation
@ -0,0 +67,4 @@
}
}
@EventHandler()
Besitzer

Wozu die Klammern?

Wozu die Klammern?
@ -0,0 +69,4 @@
@EventHandler()
public void onEntityShootBow(EntityShootBowEvent event) {
if(!(event.getEntity() instanceof Player))
Besitzer

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
@ -0,0 +75,4 @@
}
@EventHandler
public void onPlayerPickupArrow(PlayerPickupArrowEvent event) {
Besitzer

Spieler heben keine Pfeile auf (ist unterbunden)

Spieler heben keine Pfeile auf (ist unterbunden)
@ -0,0 +142,4 @@
}
private boolean checkBlock(Block block) {
return Config.HiddenBlocks.contains(blockToId(block));
Besitzer

Ich glaube, dass du nicht unbedingt die Blöcke zu ihren IDs umwandeln musst, da es ja auch die HiddenBlockTags gibt, in welchen die MATERIAL-Namen der einzelnen Blöcke drin sind, dann sparst du dir das ganze Version-Dependent-Zeugs (und ist wsl. schneller, da ein Map-Lookup weniger).

Ich glaube, dass du nicht unbedingt die Blöcke zu ihren IDs umwandeln musst, da es ja auch die HiddenBlockTags gibt, in welchen die MATERIAL-Namen der einzelnen Blöcke drin sind, dann sparst du dir das ganze Version-Dependent-Zeugs (und ist wsl. schneller, da ein Map-Lookup weniger).
Autor
Besitzer

Würde es dann auch mit block.getType().ordinal() gehen?

Würde es dann auch mit ``block.getType().ordinal()`` gehen?
Autor
Besitzer

Obwohl, Ne mit dem HiddenBlockTags ist es glaube ich auch besser.

Obwohl, Ne mit dem HiddenBlockTags ist es glaube ich auch besser.
Autor
Besitzer

Immoment könnte man es in die Produktion tuhen, sollte aber vorher wirklich noch mal die beeinflussung des Kampfgeschehens genauer begutachten um evt. das Tracking noch mal mehr an einen Fight anzupassen

Immoment könnte man es in die Produktion tuhen, sollte aber vorher wirklich noch mal die beeinflussung des Kampfgeschehens genauer begutachten um evt. das Tracking noch mal mehr an einen Fight anzupassen
Lixfel hat 2020-12-08 07:03:01 +01:00 Änderungen angefragt
@ -0,0 +55,4 @@
Entity entity = e.getKey();
if(checkBlocks(entity.getLocation().getBlock(), e.getValue().getBlock())){
entity.remove();
LAST_LOCATION.remove(entity);
Besitzer

Das müsste aus meiner Java-Erfahrung heraus auf jeden fall eine ConcurrentModificationException werfen

Das müsste aus meiner Java-Erfahrung heraus auf jeden fall eine ConcurrentModificationException werfen
@ -0,0 +60,4 @@
else {
if(entity.getTicksLived() > Config.ArrowTechhiderCollision ||
((AbstractArrow) entity).isInBlock() || entity.getLocation().equals(e.getValue())) {
LAST_LOCATION.remove(entity);
Besitzer

Ebenfalls. Nutze einen Iterator.

Ebenfalls. Nutze einen Iterator.
Lixfel hat den Titel von Arrows Stopping in Techhider Blocks zu WIP: Arrows Stopping in Techhider Blocks 2020-12-10 19:16:23 +01:00 geändert
Lixfel hat 2020-12-21 07:57:23 +01:00 Änderungen angefragt
@ -0,0 +56,4 @@
Entity entity = e.getKey();
if(checkBlocks(entity.getLocation().getBlock(), e.getValue().getBlock())){
entity.remove();
LAST_LOCATION.remove(entity);
Besitzer

Da musst du iterator.remove() verwenden.

Da musst du iterator.remove() verwenden.
@ -0,0 +59,4 @@
LAST_LOCATION.remove(entity);
}
else {
if(entity.getTicksLived() > Config.ArrowTechhiderCollision ||
Besitzer

Die Verschachtelung kann man evtl. durch eine else if-Konstruktion ersetzen.

Die Verschachtelung kann man evtl. durch eine else if-Konstruktion ersetzen.
@ -0,0 +61,4 @@
else {
if(entity.getTicksLived() > Config.ArrowTechhiderCollision ||
((AbstractArrow) entity).isInBlock() || entity.getLocation().equals(e.getValue())) {
LAST_LOCATION.remove(entity);
Besitzer

Auch hier wieder: Iterator benutzen.

Auch hier wieder: Iterator benutzen.
@ -0,0 +63,4 @@
((AbstractArrow) entity).isInBlock() || entity.getLocation().equals(e.getValue())) {
LAST_LOCATION.remove(entity);
}else {
LAST_LOCATION.replace(e.getKey(), e.getKey().getLocation());
Besitzer

Ich denke, dass du auch hier den Iterator benutzen musst, weiß aber gerade nicht, ob eine Änderung da überhaupt vorgesehen ist.

Ich denke, dass du auch hier den Iterator benutzen musst, weiß aber gerade nicht, ob eine Änderung da überhaupt vorgesehen ist.
@ -0,0 +77,4 @@
@Override
public void enable() {
super.enable();
task = Bukkit.getScheduler().runTaskTimerAsynchronously(FightSystem.getPlugin(), this::run, 1, 1);
Besitzer

Das kannst du nicht Asynchron machen (es dauert 50ms+, bis du einen Chunk für Blockabfragen bekommst), das produziert hier nur Threads.

Das kannst du nicht Asynchron machen (es dauert 50ms+, bis du einen Chunk für Blockabfragen bekommst), das produziert hier nur Threads.
Besitzer

Ist das noch WIP?

Ist das noch WIP?
Autor
Besitzer

Nope

Nope
Chaoscaot hat den Titel von WIP: Arrows Stopping in Techhider Blocks zu Arrows Stopping in Techhider Blocks 2020-12-30 00:28:51 +01:00 geändert
Lixfel hat 2020-12-30 07:32:26 +01:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Fände es auch noch schön, wenn du auch mal statt über die Zeit mal mit der Richtung versuchst, zu arbeiten (d.h. Die Flugrichtung zum Faktor machst, ob das jetzt geblockt wird oder passieren darf, finde aber, dass es wsl. auch so geht.

Fände es auch noch schön, wenn du auch mal statt über die Zeit mal mit der Richtung versuchst, zu arbeiten (d.h. Die Flugrichtung zum Faktor machst, ob das jetzt geblockt wird oder passieren darf, finde aber, dass es wsl. auch so geht.
@ -0,0 +37,4 @@
public class ArrowStopper extends BasicListener {
private BukkitTask task;
private static final HashMap<Entity, Location> LAST_LOCATION = new HashMap<>();
Besitzer

Statt der (aufwändigen) Verwaltung von LAST_LOCATIONS wäre es wsl. eleganter, mit entity.getVelocity() / 20 zu arbeiten. Das dürfte einiges an Verwaltungsaufwand sparen. Man könnte dann wsl. in run auch mit world.getEntitiesWithClass() arbeiten.

Statt der (aufwändigen) Verwaltung von LAST_LOCATIONS wäre es wsl. eleganter, mit entity.getVelocity() / 20 zu arbeiten. Das dürfte einiges an Verwaltungsaufwand sparen. Man könnte dann wsl. in run auch mit world.getEntitiesWithClass() arbeiten.
Autor
Besitzer

?, entity.getVelocity() gibt einen Vektor zurück, welchen man nicht einfach durch 20 teilen kann

?, entity.getVelocity() gibt einen Vektor zurück, welchen man nicht einfach durch 20 teilen kann
Besitzer

.divide?

.divide?
Autor
Besitzer

braucht einen Vektor

braucht einen Vektor
Besitzer

Du brauchst ja sowieso nur die einzelnen Komponenten, also teile doch einfach die durch 20....

Du brauchst ja sowieso nur die einzelnen Komponenten, also teile doch einfach die durch 20....
@ -0,0 +43,4 @@
super(EnumSet.of(FightState.RUNNING));
}
public static void init() {
Besitzer

Die Init-Methode kann gestrichen werden und alles in den Konstruktor (der dann Public wird) übernommen werden. Konstruktor wäre dann super(Config.ArrowTechhiderCollision != 0 ? EnumSet.of(RUNNING), EnumSet.empty());

Die Init-Methode kann gestrichen werden und alles in den Konstruktor (der dann Public wird) übernommen werden. Konstruktor wäre dann super(Config.ArrowTechhiderCollision != 0 ? EnumSet.of(RUNNING), EnumSet.empty());
@ -0,0 +46,4 @@
public static void init() {
if(Config.ArrowTechhiderCollision == 0)
return;
FightSystem.registerStateDependent(new ArrowStopper());
Besitzer

BasicListener übernimmt im Konstruktor schon das Registrieren als StateDependent

BasicListener übernimmt im Konstruktor schon das Registrieren als StateDependent
Lixfel hat die Änderungen 2021-01-02 08:33:13 +01:00 genehmigt
@ -0,0 +54,4 @@
prevLocation = ((Player) arrow.getShooter()).getEyeLocation();
if(checkBlocks(arrow.getLocation().getBlock(), prevLocation.getBlock())) {
arrow.remove();
iterator.remove();
Besitzer

Das dürfte nichts bringen, da der Iterator eigens auf einem für diese Aufgabe erstellter ArrayList läuft. Die Zeile drüber reicht schon vollkommen aus, um den Pfeil zu entfernen.

Das dürfte nichts bringen, da der Iterator eigens auf einem für diese Aufgabe erstellter ArrayList läuft. Die Zeile drüber reicht schon vollkommen aus, um den Pfeil zu entfernen.
@ -0,0 +80,4 @@
for (BlockFace face : BLOCK_FACES) {
Block relative = cursor.getRelative(face);
double distance = relative.getLocation().distance(end.getLocation());
if(distance < nearestDistance) {
Besitzer

Ok, wenn ich jetzt richtig verstanden habe, was diese Funktion macht, dann tut sie linear zwischen den Blöcken interpolieren? Ich meine, das ist zwar nicht die korrekte Minecraft-Berechnungsreihenfolge, soll mir aber auch recht sein.

Ok, wenn ich jetzt richtig verstanden habe, was diese Funktion macht, dann tut sie linear zwischen den Blöcken interpolieren? Ich meine, das ist zwar nicht die korrekte Minecraft-Berechnungsreihenfolge, soll mir aber auch recht sein.
@ -0,0 +97,4 @@
return Config.HiddenBlockTags.contains(block.getType().name());
}
private boolean isValidEntity(Arrow entity) {
Besitzer

Der Name der Funktion verwirrt etwas, da die Funktion ja einen Invalid boolean zurückgibt. evtl. isInvalid?

Der Name der Funktion verwirrt etwas, da die Funktion ja einen Invalid boolean zurückgibt. evtl. isInvalid?
Lixfel hat diesen Pull-Request 2021-01-02 09:20:42 +01:00 geschlossen
Lixfel löschte die Branch arrow-in-techhider 2021-01-02 09:20:55 +01:00
Dieses Repo ist archiviert. Du kannst Pull-Requests nicht kommentieren.
Keine Beschreibung angegeben.