EssentialsX

EssentialsX

2M Downloads

/tpr not respecting min-range

Bobcat00 opened this issue ยท 4 comments

commented

Information

Full output of /ess version:

Server version: 1.16.4-R0.1-SNAPSHOT git-Spigot-628435a-6917945 (MC: 1.16.4)
EssentialsX version: 2.18.2.0
LuckPerms version: 5.2.35
Vault version: 1.7.3-b131
EssentialsXChat version: 2.18.2.0
EssentialsXGeoIP version: 2.18.2.0
EssentialsXSpawn version: 2.18.2.0

EssentialsX config:

https://gist.github.com/Bobcat00/587ba22e6bd272c02436511ba56c1ea1

Details

Description:

When doing /tpr the teleport destination does not respect the min-range parameter. For example with a min-range of 2000 and a center point of 285.5 65.0 510.5, I was teleported to the following locations:
-765 69 -2627
294 69 -2445
66 71 -4212
5036 64 -906
4501 78 465
The first three have improper X coordinates, the last two have improper Z coordinates (see expected behavior below).

(To be clear, there were other destinations in the proper range.)

tpr.yml:

# Configuration for the random teleport command.
# Some settings may be defaulted, and can be changed via the /settpr command in-game.
pre-cache: false
min-range: 2000.0
excluded-biomes:
- cold_ocean
- deep_cold_ocean
- deep_frozen_ocean
- deep_lukewarm_ocean
- deep_ocean
- deep_warm_ocean
- frozen_ocean
- frozen_river
- lukewarm_ocean
- ocean
- river
- warm_ocean
max-range: 5000.0
center:
  world: LanaPug
  x: 285.5
  y: 65.0
  z: 510.5
  yaw: -359.69977
  pitch: -4.1879714E-5

Steps to reproduce:

Configure tpr.yml as shown (you'll have to specify a different world), use F3 to display the coordinates, then do /tpr multiple times. Observe the location to which you are teleported.

Expected behavior:

For the location to which you are teleported:
X should always be less than -1715 OR greater than 2285
Z should always be less than -1490 OR greater than 2510

commented

Perhaps minRange and maxRange don't mean what I think they do. @donvi-bz I think this is your code, can you explain?

private CompletableFuture<Location> calculateRandomLocation(final Location center, final double minRange, final double maxRange) {

final double rectX = RANDOM.nextDouble() * (maxRange - minRange) + minRange;
final double rectZ = RANDOM.nextDouble() * (maxRange + minRange) - minRange;

So for maxRange=5000 and minRange=2000

rectX will be in the range 2000 to 5000.
rectZ will by in the range -2000 to 5000.

That doesn't look right to me, because rectZ can have values close to 0.

commented

I don't see a problem with your coordinates. They are all at least 2000 blocks away from the center.

x is greater than 2000 blocks away from 285.5 for (5036 64 -906), and (4501 78 465)
z is greater than 2000 blocks away from 510.5 for (-765 69 -2627), (294 69 -2445), and (66 71 -4212)

If both x and z were greater than 2000 blocks and less than 5000 blocks away, your tpr area would look like this (which is incorrect, since it pushes people into "corners" rather than distributing them around the center properly):

image

The error here would be seen more and more if you set your min and max range to close or the same value as on another.

The current algorithm is designed by Tim, and was merged into Essentials after some deliberation. It actually solved previous issues with min range (as shown above) being taken into account incorrectly, and also results in a more uniform distribution of values.

You can convince yourself that it is correct by running through the numbers. As an example, let's take "min" as 2000, "max" as 5000, "rx" and "rz" as different random values in the range [0, 1).

For x: max - min = 3000, so rx * 3000 + 2000 will be in the range [0 * 3000 + 2000, 1 * 3000 + 2000) = [2000, 5000).

For z: max + min = 7000, so rz * 7000 - 2000 will be in the range [0 * 7000 - 2000, 1 * 7000 - 2000) = [-2000, 5000).

Together these encompass the rectangular region between [2000, -2000] and (5000, 5000). Indeed this has a Z value which can be close to zero, but is actually still outside of the square defined by a radius of min from the center.

Then you still need to apply one of four rotation matrices to both points to achieve the final region (which is the rectangular donut shape that you expect between min and max range). These are simplified in the code for efficiency, as they follow a pattern.

Transformation 1:
point 1: [(2000)*cos(0*pi) - (-2000)*sin(0*pi), (2000)*sin(0*pi) + (-2000)*cos(0*pi)] -> [2000, -2000]
point 2: ((5000)*cos(0*pi) - (5000)*sin(0*pi), (5000)*sin(0*pi) + (5000)*cos(0*pi)) -> (5000, 5000)
simplification: x, z -> x, z (it's not rotated)

Transformation 2:
point 1: [(2000)*cos(0.5*pi) - (-2000)*sin(0.5*pi), (2000)*sin(0.5*pi) + (-2000)*cos(0.5*pi)] -> [2000, 2000]
point 2: ((5000)*cos(0.5*pi) - (5000)*sin(0.5*pi), (5000)*sin(0.5*pi) + (5000)*cos(0.5*pi)) -> (-5000, 5000)
simplification: x, z -> -z, x

Transformation 3:
point 1: [(2000)*cos(1*pi) - (-2000)*sin(1*pi), (2000)*sin(1*pi) + (-2000)*cos(1*pi)] -> [-2000, 2000]
point 2: ((5000)*cos(1*pi) - (5000)*sin(1*pi), (5000)*sin(1*pi) + (5000)*cos(1*pi)) -> (-5000, -5000)
simplification: x, z -> -x, -z

Transformation 4:
point 1: [(2000)*cos(1.5*pi) - (-2000)*sin(1.5*pi), (2000)*sin(1.5*pi) + (-2000)*cos(1.5*pi)] -> [-2000, -2000]
point 2: ((5000)*cos(1.5*pi) - (5000)*sin(1.5*pi), (5000)*sin(1.5*pi) + (5000)*cos(1.5*pi)) -> (5000, -5000)
simplification: x, z -> z, -x

After applying one of these transformations randomly, the distribution looks like this visually:

image

The final point which was contained within the original region and transformed is then taken as the offset from the center.

commented

I don't see a problem with your coordinates. They are all at least 2000 blocks away from the center.
If both x and z were greater than 2000 blocks and less than 5000 blocks away, your tpr area would look like this (which is incorrect, since it pushes people into "corners" rather than distributing them around the center properly):

Ah, you're right! For some reason, that was not obvious to me. So this can be closed and I'll withdraw the PR if I can figure out how.

Thank you for the explanation.

commented

No problem, just wanted to actually make sure there wasn't a bug here (and document this because I realized it is written no where). Glad to explain and clear up any doubts.