feat: Add more options for ping passthrough#1631
feat: Add more options for ping passthrough#1631ButterDebugger wants to merge 17 commits intoPaperMC:dev/3.0.0from
Conversation
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev>
…through-dev) * Added 'ALLBUTVERSION' option for ping passthrough. * Trying to get the GitHub build action to run * Added more configuration options for ping passthrough. * Updated default velocity.toml * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <cas@mizule.dev> * Add support for the legacy ping passthrough. --------- Co-authored-by: TheMiningTeamYT <loanisamazing@outlook.com> Co-authored-by: powercas_gamer <cas@mizule.dev>
- Bumped config-version to 2.8 - Updated comments and grammar - Removed legacy ping passthrough in the config
|
I'll be honest, after several months of nothing happening I gave up on my pull request ever getting merged, so I stopped paying attention XD. |
| if (mode.version) { | ||
| version = response.getVersion(); | ||
| } else { | ||
| version = fallback.getVersion(); | ||
| } |
There was a problem hiding this comment.
Ternaries for each of those would make them a bit more compact. And not sure if this means much, but this looks functionally different to the description and mods search from before where it would continue iterating responses if they're null in the current one
There was a problem hiding this comment.
On the ternaries, personal preference but that's an easy fix to make down the road. I don't think that should keep you from merging the pull request.
As for the difference in handling mod/description search: If you were to implement that in this code, you'd run the risk of, say, returning the player count from one server and the mod list from another, or returning the player count & mod list from a different server from the one the player is about to join, so unless someone jumps in here and says "hey I need that," I think it's fine the way it is.
| # configuration is used if no servers could be contacted. | ||
| ping-passthrough = "DISABLED" | ||
| # Should Velocity pass the version number from the backend server when responding to server list ping requests? | ||
| ping-passthrough-version = false |
There was a problem hiding this comment.
This should be its own section rather than having the same prefix in the root section
There was a problem hiding this comment.
I agree. When I wrote this code, I did not know that was a thing you could do. Had I known, I would've done so.
3 participants
Heyo,
This is an updated version of this pull request with added config migrations and removal of dead code. The original author of these changes doesn't appear to be responsive as of late, so I've made a new pull request with my additional changes. Please let me know if there is anything you would like me to modify or add, I'd be happy to make any code changes to hopefully see this get merged.
Thank you :D