-
-
Notifications
You must be signed in to change notification settings - Fork 407
Add worldborders #7006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add worldborders #7006
Conversation
src/main/java/ch/njol/skript/expressions/ExprWorldBorderSize.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TEST!
src/main/java/ch/njol/skript/conditions/CondIsInsideWorldBorder.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondIsInsideWorldBorder.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondIsInsideWorldBorder.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderSize.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderSize.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningDistance.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningTime.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningTime.java
Outdated
Show resolved
Hide resolved
A lot of chez's stuff might of been repeated he beat me ;( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just needs some formatting updates, mainly around annotations!
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderSize.java
Outdated
Show resolved
Hide resolved
2cdbc4e
to
d7dbc0c
Compare
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningTime.java
Outdated
Show resolved
Hide resolved
import org.jetbrains.annotations.Nullable; | ||
|
||
@Name("Warning Time of World Border") | ||
@Description("The warning time of a world border. If the border is shrinking, the player's screen will be tinted red once the border will catch the player within this time period.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if the player is outside the border it will be tinted red after one second? im not sure what this expression does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its specifically when the worldborder is shrinking. If the player is outside the final worldborder (after the shrink), their screen will turn red x seconds before it reaches their current location
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningTime.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few things
src/main/java/ch/njol/skript/expressions/ExprWorldBorderCenter.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderDamageAmount.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderDamageBuffer.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderSize.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderDamageAmount.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderCenter.java
Outdated
Show resolved
Hide resolved
Location location = mode == ChangeMode.SET ? (Location) delta[0] : null; | ||
for (WorldBorder worldBorder : getExpr().getArray(event)) { | ||
switch (mode) { | ||
case SET: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big deal, but maybe an enhanced switch?
499ff66
to
e6eef8b
Compare
…order#getMaxCenterCoordinate
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a proper review to other people, but uhh thoughts on WorldBorder x AnySizedThing? size of worldborder
, set size of worldborder to 10
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
@@ -53,6 +53,7 @@ | |||
import org.bukkit.event.player.PlayerQuitEvent.QuitReason; | |||
import org.bukkit.event.player.PlayerResourcePackStatusEvent.Status; | |||
import org.bukkit.event.player.PlayerTeleportEvent.TeleportCause; | |||
import org.bukkit.event.player.PlayerExpCooldownChangeEvent.ChangeReason; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you've had some trouble merging/updating your branch, so let's hope nothing got removed in the making.
src/main/java/ch/njol/skript/expressions/ExprSecCreateWorldBorder.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprSecCreateWorldBorder.java
Outdated
Show resolved
Hide resolved
if (((long) worldBorder.getWarningDistance() + input) > Integer.MAX_VALUE) { | ||
worldBorder.setWarningDistance(Integer.MAX_VALUE); | ||
} else if (((long) worldBorder.getWarningDistance() + input) < Integer.MIN_VALUE) { | ||
worldBorder.setWarningDistance(Integer.MIN_VALUE); | ||
} else { | ||
worldBorder.setWarningDistance(Math.max(worldBorder.getWarningDistance() + input, 0)); | ||
} | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (((long) worldBorder.getWarningDistance() + input) > Integer.MAX_VALUE) { | |
worldBorder.setWarningDistance(Integer.MAX_VALUE); | |
} else if (((long) worldBorder.getWarningDistance() + input) < Integer.MIN_VALUE) { | |
worldBorder.setWarningDistance(Integer.MIN_VALUE); | |
} else { | |
worldBorder.setWarningDistance(Math.max(worldBorder.getWarningDistance() + input, 0)); | |
} | |
break; | |
int current = worldBorder.getWarningDistance(); | |
int value = Math2.fit(0, current + input, Integer.MAX_VALUE); | |
worldBorder.setWarningDistance(value) |
Shouldnt be using MIN_VALUE
because that's a negative, I dont think a world border should have a negative distance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MIN_VALUE part is true but I did the rest of it to catch integer overflows during the addition / subtraction. It seems like the Math2.fit wouldn't help that unless I am misunderstanding the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well one thing you should do, is in #acceptChange
instead of returning Number.class
you should do Integer.class
in my opinion. As that in itself would exclude any nan and infinite values. As well as capping the provided number to Integer.MAX_VALUE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think switching it to integer class fixed the integer overflow issues so I think it might leave it as is with the MIN_VALUE thing fixed
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningDistance.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWorldBorderWarningTime.java
Outdated
Show resolved
Hide resolved
src/test/skript/tests/syntaxes/sections/ExprSecCreateWorldBorder.sk
Outdated
Show resolved
Hide resolved
Co-authored-by: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com>
What do you mean? Is this some newer feature that I haven't seen yet / forgot about? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
i appreciate the very thorough tests
Description
Add worldborder syntax to Skript. Continuation of #5229
Player's worldborder is always virtual
Conditions:
Effects:
I don't like the way I am registering the syntax right now with the
of
and's
but I can't think of a different wayEvents:
Expressions:
Sections:
Target Minecraft Versions: any
Requirements: Paper for worldborder events
Related Issues: #5178, #3848