Repository navigation
Fix dimension suggestions for dynamically loaded worlds - #14399
Gabrieljr42 wants to merge 4 commits into
Conversation
|
Btw, I just found this edge case: if you toggle the option using /paper reload, players already online will keep the previous behavior until they reconnect or their command tree is resent. I added a note about this in the config comment. |
|
A better solution would be to resend players into the config phase on each world load & unload. It would actually fix the underlying issue which is a mismatch between the registry contents on the client and the server, and not patch over it. But from what i know, Bukkit (and now Paper) API wasn’t exactly designed with player objects not being stable across the connection lifecycle in mind, so this could possibly create compatibility issues with plugins not expecting the player instance to be destroyed and recreated while the player is still connected. As this PR stands, it only paints over the true issue, and while working, it is brittle and paves the way for more issues, should Mojang change anything. |
For reference, i have a project which does the exact reconfigure solution using the |
Thanks for taking a look. Yeah, that makes sense. This PR only covers the autocomplete gap, it doesn’t really fix the registry mismatch. |
|
Thanks @Toffikk, I reworked this around the configuration phase in 27b3359. It now resends registries and the current world list after worlds load or unload, with changes grouped until the end of the tick. The previous command-suggestion workaround is removed. The new option is disabled by default because this fires quit/join events and creates new Player objects. It skips legacy PlayerLoginEvent connections and leaves plugin-controlled reconfiguration alone. The 17 tests pass, the test plugin builds, and the patches apply fine. I’m working on this in my free time and haven’t had a chance to try it in-game yet, so plugin compatibility still needs checking. The reload edge case I mentioned earlier no longer applies. |
Warriorrrr
left a comment
There was a problem hiding this comment.
Sending all players to the configuration stage for something small like updating a command suggestion really does not seem worth it for how intrusive it is, and I'd have concerns with plugin compat when it's intentionally skipping calling the reconfigure event.
The initial solution that just turns it into an ask server is good enough to me, the tests it's added should be removed since they're not really useful and mock internals to quite a degree.
Sending players through configuration to refresh a command suggestion is too intrusive, and skipping the reconfigure event is a plugin compatibility problem. Keep the opt-in ask-server provider and remove the tests that mock command internals.
|
Thanks @Warriorrrr. I dropped the configuration phase path and went back to the ask-server option in 77cf97a. The tests are gone too, including the ones that mocked the command internals and the test plugin command. |
Warriorrrr
left a comment
There was a problem hiding this comment.
looks good to me, please also create a PR in our docs repository to document the addition of this new option if you're able
|
Thanks @Warriorrrr! I opened the documentation PR: PaperMC/docs#818. It covers the option’s default value, behavior, and the reconnect or command-tree refresh requirement after changing it. Thanks for the review! |
|
Is it correct to default this to off? What is the potential downside of enabling this by default while allowing disabling it? It seems to me that starting out enabled is the preferred behavior by almost everyone. |
One trade-off is that these completions require a server round trip, so network latency can affect how quickly suggestions appear. Keeping it off preserves the existing client-side behavior. I also see the benefit of enabling it by default: players would get up-to-date world suggestions without server owners having to discover and enable the setting. @Warriorrrr, since you reviewed this version, what do you think about enabling it by default? I'll defer to you on that and update this PR and the companion docs PR accordingly. |
Fixes #13737.
Dimension arguments suggest from the world list the client got at login, so worlds created through the API after that do not show up until the player rejoins.
commands.server-side-dimension-suggestionsis off by default. When it is on, a dimension argument with no suggestion provider of its own is sent asminecraft:ask_server. Tab completion then uses the server's current world list, so worlds loaded after join are included and unloaded worlds are not. Players have to reconnect, or have their command tree resent, after this setting changes. Arguments that already set a suggestion provider are left as they are.This does not send players back through the configuration phase.
Testing with a real Minecraft client is still pending.
I used AI assistance for the implementation.