-
-
Notifications
You must be signed in to change notification settings - Fork 120
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 the ctick command #712
base: fabric
Are you sure you want to change the base?
Conversation
src/main/java/net/earthcomputer/clientcommands/command/CTickCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/net/earthcomputer/clientcommands/command/CTickCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/net/earthcomputer/clientcommands/command/CTickCommand.java
Outdated
Show resolved
Hide resolved
|
||
@Override | ||
public boolean condition() { | ||
return tickCount <= 1200; |
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 wasn't a problem in 1.12 since the client tick rate was always expected to be 20 and hardly ever fell below 20, but now if the TPS is set to 1 then this could take a long time to complete. I would suggest using real time instead. Just be extra careful about there being zero ticks elapsed and dividing by zero.
currentMeasurer = measurer; | ||
|
||
float tps = source.getWorld().tickRateManager().tickrate(); | ||
source.sendFeedback(Component.translatable("commands.ctick.client.tps.expectedTps", tps)); |
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.
These expected TPS and MSPT messages shouldn't be printed while the game is sprinting
stopPreviousTask(); | ||
|
||
boolean isIntegratedServer = source.getClient().hasSingleplayerServer(); | ||
TickMeasuringTask measurer = new TickMeasuringTask(false, !isIntegratedServer); |
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'm not seeing where there is any accurate measurements from the integrated server. Be careful about thread safety with that, though.
long totalTime = lastTickStart - firstTickStart; | ||
double mspt = totalTime / (1_000_000D * tickCount); | ||
minecraft.player.displayClientMessage(Component.translatable("commands.ctick.mspt", DEC_FMT.format(mspt)), false); | ||
minecraft.player.displayClientMessage(Component.translatable("commands.ctick.mspt.inaccurate"), false); |
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 message shouldn't display if the server is clearly lagging (e.g. the observed TPS is say 10% lower than expected). It also shouldn't display if the game is sprinting. Also for cosmetic reasons maybe it could be replaced with one of those help comments (ClientCommandHelper.sendHelp
)
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 was copied over from your original code which I wanted to stay as close to as possible.
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 concept of help messages didn't exist back then, and besides we can improve on code I wrote 7 years ago :)
This PR adds a simple
ctick
command for querying both the client's and server's TPS and MSPT. The expected value is printed for completeness. The task code is mostly copied from:clientcommands/src/main/java/net/earthcomputer/clientcommands/command/CommandTick.java
Lines 175 to 255 in c1e3766