Skip to content

Conversation

@Lumine1909
Copy link
Contributor

The change is such simple
As the commit message, the origin method can not get the correct UUID in offline minecraft server and cause a series of problem (such as can't delete builders)
So we just need a judge of if server is offline mode : )

@thomasmny
Copy link
Owner

Have you tested this behind a proxy? Servers there are also set to offline mode but AFAIK the UUID isn't created that way

@Lumine1909
Copy link
Contributor Author

Have you tested this behind a proxy? Servers there are also set to offline mode but AFAIK the UUID isn't created that way

CraftBukkit use this method internally to get the offline uuid of a player.

I haven't tested it on proxy server, perhaps it won't work correctly, I will change the impl to Bukkit#getOfflinePlayer, which can get the correct uuid when proxy online and subserver offline.

@Lumine1909
Copy link
Contributor Author

I delete almost all of the part of UUIDFetcher. I think I have to explain the reason.

Firstly, all the usage of UF is "Find some player with specific uuid or name who ever played in the server". That means we aren't need to find the player's info from Mojang API, the local cache is enough (usercache.json in the root folder of server).

Secondly, CraftBukkit gives a good implementation of find online/offline.

    public OfflinePlayer getOfflinePlayer(String name) {
        Preconditions.checkArgument(name != null, "name cannot be null");
        Preconditions.checkArgument(!name.isBlank(), "name cannot be empty");
        OfflinePlayer result = this.getPlayerExact(name);
        if (result == null) {
            GameProfile profile = null;
            if (this.getOnlineMode() || GlobalConfiguration.get().proxies.isProxyOnlineMode()) {
                profile = (GameProfile)this.console.getProfileCache().get(name).orElse((Object)null);
            }

            if (profile == null) {
                result = this.getOfflinePlayer(new GameProfile(UUID.nameUUIDFromBytes(("OfflinePlayer:" + name).getBytes(Charsets.UTF_8)), name));
            } else {
                result = this.getOfflinePlayer(profile);
            }
        } else {
            this.offlinePlayers.remove(((OfflinePlayer)result).getUniqueId());
        }

        return (OfflinePlayer)result;
    }

All of the condition are considered.

So the deleted part are not necessary.

@thomasmny
Copy link
Owner

I'll test your branch on a few different setups (online, offline, proxy, etc.) and if non of them fail, then I'll merge your code

@Lumine1909
Copy link
Contributor Author

I'll test your branch on a few different setups (online, offline, proxy, etc.) and if non of them fail, then I'll merge your code

OK
If there is anything wrong, please reach out to me. Thank you!

@thomasmny
Copy link
Owner

Sorry it took so long, but I have a lot of work to do right now.

I've tested this build with a proxy and it still works. However, I think we should keep the UUID- & NameCache since we otherwise always read the data from disk.

@Lumine1909
Copy link
Contributor Author

Sorry it took so long, but I have a lot of work to do right now.

I've tested this build with a proxy and it still works. However, I think we should keep the UUID- & NameCache since we otherwise always read the data from disk.

Mojang is already create a cache for checks, hold another cache by ourselves is unnecessary.

image

@thomasmny
Copy link
Owner

Weird, definitely doesn't feel like it.
If I have an offline server behind a proxy where the target player is offline, then the process takes noticeable time in comparison to when the player is online

@Lumine1909
Copy link
Contributor Author

Weird, definitely doesn't feel like it. If I have an offline server behind a proxy where the target player is offline, then the process takes noticeable time in comparison to when the player is online

Could you tell me how to find the true time of find the player, I will try it on my environment and find some deeply reason.

@thomasmny
Copy link
Owner

I timed it using

public static UUID getUUID(String name) {
    long start = System.currentTimeMillis();
    UUID uuid = Bukkit.getOfflinePlayer(name).getUniqueId();
    Logger.getLogger("UUIDFetcher").info("Fetching UUID for " + name + " took " + (System.currentTimeMillis() - start) + "ms");
    return uuid;
}

but for whatever reason, it works much better now. The first call is slow (expected), however subsequent work just fine. Will want to test a tiny bit more but so far it looks good

@thomasmny thomasmny merged commit a00d89f into thomasmny:master Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants