[Feature, Improvement]: Disable converging, prevent IllegalArgumentException #141

DerToaster98 · 2024-11-24T17:23:33Z

Converging automatically toggles off, if the range is less than 0. We tried setting it to 0 with the old version and it still tried to converge. Specifying a long list of blocks that are "transparent" is not really a option if you dont want converging
Our console used to get spammed with "X is not finite" exceptions. Clamped the vectors now so it does not run into that try block as printing out all those exceptions slows the server down.

Revert "Add APDevTeam#118 + Small refactor (APDevTeam#135)" (APDevTeam#140)

DerToaster98 · 2024-11-24T17:45:29Z

Need to do some more testing, looks like i broke directors with the clamping

(cherry picked from commit acce226)

TylerS1066 · 2024-12-14T19:55:16Z

Need to do some more testing, looks like i broke directors with the clamping

Every time changes are attempted it breaks things, I am hesitant to accept changes here.

DerToaster98 · 2024-12-14T20:06:45Z

Need to do some more testing, looks like i broke directors with the clamping

Every time changes are attempted it breaks things, I am hesitant to accept changes here.

For your information, that has been fixed in the following commit. That patch is live on TTE since 3 weeks ago and has seen a lot of combat since then. Works just fine.

DerToaster98 · 2024-12-14T20:18:46Z

If you are intersted, the part that broke it is explained in the commit "correct mathhelper". Basically Double.MIN_VALUE is not what one expects at first glance, instead it is the lowest positvice double value to match IEEE754 format.

What the function does overall is limiting the value between the min and max limits, which prevents the "not finite" exception from occuring

TylerS1066 · 2024-12-21T02:23:42Z

src/main/java/net/countercraft/movecraft/combat/features/directors/AADirectors.java

-        else { // shoot directly at the block the player is looking at (IE: with convergence)
-            targetVector = targetBlock.getLocation().toVector().subtract(fireball.getLocation().toVector());
-            targetVector = targetVector.normalize();
+        if (AADirectorRange >= 0) {


What is the point of this check? If you mean to check the configuration, that should be done once on load not on every fireball.

It is a less than comparison. Sure, i could cache it in a boolean value, but that isn't really a difference. Point is to be able to actually disable converging completely. With the "original" version of yours that is NOT possible (tried setting it to negative values and 0). The only "proper" way to disable converging on your version is defining a long list of blocks that are "transparent", not really clean to manage and unnecessary. Plus, if the distance is less than or equals to 0, performing the raycast is completely unnecessary.

TylerS1066 · 2024-12-21T02:24:12Z

src/main/java/net/countercraft/movecraft/combat/features/directors/AADirectors.java

+            Block targetBlock = DirectorUtils.getDirectorBlock(p, AADirectorRange);
+            if (targetBlock != null && !targetBlock.getType().isAir()) {
+                targetVector = targetBlock.getLocation().toVector().subtract(fireball.getLocation().toVector());
+                targetVector = targetVector.normalize();
+            }


What is the purpose of changing this code for no effect?

Because DirectorUtils will ALWAYS execute a raycast, regardless of what you give it, sure the relevant code here could be included there however.

TylerS1066 · 2024-12-21T02:26:37Z

src/main/java/net/countercraft/movecraft/combat/features/directors/AADirectors.java

-        Block targetBlock = DirectorUtils.getDirectorBlock(p, AADirectorRange);
-        Vector targetVector;
+        // the player is looking at nothing, shoot in that general direction
+        Vector targetVector = p.getLocation().getDirection();


Because DirectorUtils will ALWAYS execute a raycast, regardless of what you give it, sure the relevant code here could be included there however.

TylerS1066 · 2024-12-21T02:26:59Z

src/main/java/net/countercraft/movecraft/combat/features/directors/ArrowDirectors.java

+        // the player is looking at nothing, shoot in that general direction
+        Vector targetVector = p.getLocation().getDirection();
+        if (ArrowDirectorRange >= 0) {
+            Block targetBlock = DirectorUtils.getDirectorBlock(p, ArrowDirectorRange);
+            if (targetBlock != null && !targetBlock.getType().equals(Material.AIR)) {
+                // shoot directly at the block the player is looking at (IE: with convergence)
+                targetVector = targetBlock.getLocation().toVector().subtract(arrow.getLocation().toVector());
+                targetVector = targetVector.normalize();
+            }


Because DirectorUtils will ALWAYS execute a raycast, regardless of what you give it, sure the relevant code here could be included there however.

TylerS1066 · 2024-12-21T02:27:07Z

src/main/java/net/countercraft/movecraft/combat/features/directors/CannonDirectors.java

+        // the player is looking at nothing, shoot in that general direction
+        Vector targetVector = p.getLocation().getDirection();
+
+        if (CannonDirectorRange >= 0) {
+            Block targetBlock = DirectorUtils.getDirectorBlock(p, CannonDirectorRange);
+            if (targetBlock != null && targetBlock.getType().equals(Material.AIR)) {
+                // shoot directly at the block the player is looking at (IE: with convergence)
+                targetVector = targetBlock.getLocation().toVector().subtract(tnt.getLocation().toVector());
+            }
+        }


Because DirectorUtils will ALWAYS execute a raycast, regardless of what you give it, sure the relevant code here could be included there however.

TylerS1066 · 2024-12-21T02:28:33Z

src/main/java/net/countercraft/movecraft/combat/utils/MathHelper.java

+    public static float clamp(float value) {
+        return clamp(-Float.MAX_VALUE, Float.MAX_VALUE, value);
+    }
+
+    public static int clamp(int value) {
+        return clamp(Integer.MIN_VALUE, Integer.MAX_VALUE, value);
+    }
+
+    public static double clamp(double min, double max, double value) {
+        if (value > max) {
+            return max;
+        }
+        if (value < min) {
+            return min;
+        }
+        return value;
+    }
+
+    public static int clamp(int min, int max, int value) {
+        if (value > max) {
+            return max;
+        }
+        if (value < min) {
+            return min;
+        }
+        return value;
+    }
+
+    public static float clamp(float min, float max, float value) {
+        if (value > max) {
+            return max;
+        }
+        if (value < min) {
+            return min;
+        }
+        return value;
+    }


All of this should be replaced with one single utility function in Directors.java which clamps an entire vector.

It is not director specific code. It is math. Thus one does not expect it to be in DirectorUtils, that'y why it is in a separate MathHelper class cause it makes more sense there logically. I can add a function to clamp an entire vector.

Functions should be where they thematically fit.

(cherry picked from commit f5628f7)

(cherry picked from commit dd99d56)

DerToaster98 added 4 commits November 24, 2024 18:03

Merge pull request #2 from APDevTeam/main

1fa5c1c

Revert "Add APDevTeam#118 + Small refactor (APDevTeam#135)" (APDevTeam#140)

if director range is less than 0, do not try convering

91f3bda

implement clamp methods

db16481

clamp vectors

8017b91

DerToaster98 marked this pull request as draft November 24, 2024 17:45

correct mathhelper

88dd7e2

(cherry picked from commit acce226)

DerToaster98 marked this pull request as ready for review November 24, 2024 18:10

TylerS1066 requested changes Dec 21, 2024

View reviewed changes

DerToaster98 added 4 commits December 24, 2024 13:26

add single method for clamping vectors

98523e9

(cherry picked from commit f5628f7)

fix stackoverflow exception

26c222a

(cherry picked from commit dd99d56)

use clampVector method

f4fa334

do the same for arrow and cannons director

825f711

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature, Improvement]: Disable converging, prevent IllegalArgumentException #141

[Feature, Improvement]: Disable converging, prevent IllegalArgumentException #141

DerToaster98 commented Nov 24, 2024

DerToaster98 commented Nov 24, 2024

TylerS1066 commented Dec 14, 2024

DerToaster98 commented Dec 14, 2024

DerToaster98 commented Dec 14, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

TylerS1066 Dec 21, 2024

DerToaster98 Dec 21, 2024

[Feature, Improvement]: Disable converging, prevent IllegalArgumentException #141

Are you sure you want to change the base?

[Feature, Improvement]: Disable converging, prevent IllegalArgumentException #141

Conversation

DerToaster98 commented Nov 24, 2024

DerToaster98 commented Nov 24, 2024

TylerS1066 commented Dec 14, 2024

DerToaster98 commented Dec 14, 2024

DerToaster98 commented Dec 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment