Add getLastPing to PlayerCommonConnection#13552
Add getLastPing to PlayerCommonConnection#13552Cryptite wants to merge 4 commits intoPaperMC:mainfrom
Conversation
|
Would it make sense to return |
|
It would seem weird for that to be a Duration while getPing() right next to it is in ms, plus ms seems to be the generally accepted resolution on "the internets." But it's easy enough to change if there's more demand for it I suppose. I just assume that even if it was a Duration, the absolute majority use-case would be people converting it to milliseconds. |
lynxplay
left a comment
There was a problem hiding this comment.
We hardforked, no need for comments.
Owen1212055
left a comment
There was a problem hiding this comment.
This should not be on Player, rather the PlayerCommonConnection.
Also include some of the other ping stuff.
Doing a cursory investigation of this this morning, moving the existing latency/keepalive stuff here would turn this into a not-very-trivial PR. If this is quite desired I can try it but to relocate the existing ping stuff down into |
|
What you mean is that you probably want to rebase onto the files PR to do the change provisionally against the vanilla keepalive impl, and then reapply all of the feature patches and make the tweak to the keepalive improvement patch |
This was done already with Owen/Lynxplay's help yesterday, I was just referring to Owen's request to also relocate ping related logic. |
|
They mean to promote the few existing ping related methods in Player up to the connection API class, not sure if there is some invisible complexity (not looked at specifics) there or you misread into something? |
This exposes most recent ping in milliseconds on a Player. The default
Player.getPing()represents an average over time, but it is very nice to be able to actually see their most recent ping.