Skip to content
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

Set player locale when creating Audience #160

Merged
merged 2 commits into from
Jun 2, 2024
Merged

Conversation

CubBossa
Copy link
Contributor

@CubBossa CubBossa commented Mar 28, 2024

The pull request sets the client locale for Player audiences in the adventure-platform-bukkit module.
Until now, the client locale was only correct if the player changed their locale via settings while playing on the server.

@CubBossa
Copy link
Contributor Author

So it is running fine on 1.12 up to 1.20.4, can't tell about 1.20.5 yet.
1.8 has been tested but does not work, it instead returns system language via Locale.getDefault()
Anything I can do to get this merged?

@zml2008
Copy link
Member

zml2008 commented May 7, 2024

Why do i feel like on 1.8 getLocale() was on .spigot() or something? is that just me making things up?

@zml2008
Copy link
Member

zml2008 commented May 8, 2024

oh yes, https://hub.spigotmc.org/stash/projects/SPIGOT/repos/spigot/browse/Bukkit-Patches/0018-Implement-Locale-Getter-for-Players.patch?at=e4d4710834f7b292bc496f41f1cf9ac363225c0e#21

Any chance you'd be able to revise this PR to include that variant? I'd love to spin off a -platform release soon including this PR.

@CubBossa
Copy link
Contributor Author

CubBossa commented May 8, 2024

.spigot().getLocale() doesn't seem to work properly either. In forums I read that for 1.8 you'd have to listen to the settings packet. The PR removed the event listening and replaced it with a direct getLocale call on the player object, but the event didn't even exist on 1.8 and has been introduced in 1.12 I think? So either way it'd be an improvement already but doesn't fix the 1.8 issues yet

At least, the following code did not work, it still returns Locale.getDefault():

    private static final MethodHandle SPIGOT_SUPPORTED = findMethod(Player.class, "spigot", findClass("org.bukkit.entity.Player.Spigot"));
    private static final MethodHandle SPIGOT_LOCALE_SUPPORTED = findMethod(findClass("org.bukkit.entity.Player.Spigot"), "getLocale", String.class);

//...

      builder.withDynamic(Identity.LOCALE, () -> {
        if (LOCALE_SUPPORTED != null) {
          try {
            return Translator.parseLocale((String) LOCALE_SUPPORTED.invoke(viewer));
          } catch (final Throwable error) {
            logError(error, "Failed to call getLocale() for %s", viewer);
          }
        }
        if (SPIGOT_SUPPORTED != null && SPIGOT_LOCALE_SUPPORTED != null) {
          try {
            final String localeStr = (String) SPIGOT_LOCALE_SUPPORTED.invoke(SPIGOT_SUPPORTED.invoke(viewer));
            return Translator.parseLocale(localeStr);
          } catch (final Throwable error) {
            logError(error, "Failed to call spigot().getLocale() for %s", viewer);
          }
        }
        return Locale.getDefault();
      });

@zml2008
Copy link
Member

zml2008 commented Jun 2, 2024

You know, if 1.8 is fixable then let's just let the 1.8 people PR that fix and get this release out already, otherwise the PR looks fine

@zml2008 zml2008 self-assigned this Jun 2, 2024
@zml2008 zml2008 added type: bug Something isn't working platform: bukkit labels Jun 2, 2024
@zml2008 zml2008 merged commit 8969272 into KyoriPowered:main Jun 2, 2024
3 checks passed
@zml2008 zml2008 added this to the 4.3.3 milestone Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: bukkit type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants