-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Adds support for modifying the tick rate #6592
base: dev/feature
Are you sure you want to change the base?
Conversation
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.
This also needs tests for the parts that can be tested. (probably can't test stepping, maybe with junit)
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.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 minor stuff
will test tomorrow
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
|
||
@Override | ||
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { | ||
if (!parseResult.tags.isEmpty()) { |
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.
this should always be true (so it can be removed)
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprServerTickRate.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprServerTickRate.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprServerTickRate.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/conditions/CondServerTickState.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFrozenTicksToRun.java
Outdated
Show resolved
Hide resolved
}) | ||
@Since("INSERT VERSION") | ||
@RequiredPlugins("Minecraft 1.20.4+") | ||
public class ExprServerTickRate extends SimpleExpression<Float> { |
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.
Is there any connection between this and tps? I can see them getting mixed up.
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 existing TPS expression gets the TPS over a period of time (1, 5, 15 minutes)
This one gets the current tick rate, which is most likely more precise then the existing expression, its also more specific to tick rate management, as opposed to average TPS over intervals.
though maybe this should be an option of that expression instead of something entirely new? Would love to hear other peoples thoughts
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.
Though, if it were to get added to that expression, I feel it would be tricky to come up with good syntax for it as [the] tps
already exists in that expression, and returns all of the tps over the intervals.
putting this into that expression would output some dodgy syntax i feel like, I personally think it'd be best to keep them separate, but like I said before, would love to hear some other peoples thoughts
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.
No this doesn't get a more precise tps
It gets/sets the goal tps, which by default is 20. Your server can be having 10 tps but be targetting 20 tps, therefore lagging. It can have 5 tps and target 5 tps, therefor not lagging. TPS would return 10 and 5 respectively, but this expression would return 20 and 5.
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.
No this doesn't get a more precise tps It gets/sets the goal tps, which by default is 20. Your server can be having 10 tps but be targetting 20 tps, therefore lagging. It can have 5 tps and target 5 tps, therefor not lagging. TPS would return 10 and 5 respectively, but this expression would return 20 and 5.
whoops 😅, then yeah, this should stay its own expression
Description
This PR aims to add support for the tick rate to be changed, modified, etc, etc
Currently unfinished.
Plans to add a lot more to this.
Target Minecraft Versions: 1.20.3+
Requirements: none
Related Issues: none