QOL changes

[Andre601]

This is related to #ticket-0119 on the Discord Server.

I tried implementing a very basic counter for tried connections that stops scheduling future attempts after 3 failed attempts.

It doesn't look that good but at least is something worth trying.

Additionally did I create a separate private method to retrieve the byte array for the head info to reduce duplicate code and also made the LinkedHashTreeMap have <?, ?> options to fix "Raw usage" warnings in the IDE. If those changes are unwanted, let me know and I'll revert them...

[CodeCrafter47]

Thanks for the PR.

The bug description in the ticket is that the plugin will send an exponentially increasing number of requests. That is not intended. The intended behavior is that there is one request per icon every five minutes. I don't like the proposed fix, because it adds a work-around to the problem instead of fixing the root cause. To fix the issue we should investigate why there is more than one request per icon every five minutes.

Besides any bugs in the code I'm aware of the following two situations, that can increase the number of requests being sent.

• Use of the /btlp reload command.
• Repeatedly creating an Icon from the same image using the BTLP API.

I don't know if any of these apply here.

[Andre601]

I removed the connection counter stuff, but left in the other QOL changes since it reduces duplicate code to maintain and also helps a bit with IDE warnings.

I would also go and suggest the switch from Google's cache to Ben-manes Caffeine Cache. From the looks of it are they virtually the same in syntax and look, but Caffeine is not "unstable" like Google and worked reliably for me in the past.

Additionally does Caffeine have a get method to execute a method directly when no value is present (Has expired) allowing to get rid of a lot getIfPresent calls followed by null checks.

Example:

    // Before: Get Entry, check if null and compute stuff when null
    private synchronized CompletableFuture<Icon> fetchIcon(UUID uuid) {
        CompletableFuture<Icon> future = cacheUUID.getIfPresent(uuid);

        if (future == null) {
            future = new CompletableFuture<>();
            CompletableFuture<Icon> finalFuture = future;
            asyncExecutor.execute(() -> fetchIconFromMojang(uuid, finalFuture));
            cacheUUID.put(uuid, future);
        }
        return future;
    }

    // After: Get entry and compute if absent (null)
    private synchronized CompletableFuture<Icon> fetchIcon(UUID uuid) {
        return cacheUUID.get(uuid, k -> {
            CompletableFuture<Icon> future = new CompletableFuture<>();
            CompletableFuture<Icon> finalFuture = future;
            asyncExecutor.execute(() -> fetchIconFromMojang(uuid, finalFuture));

            return future;
        });
    }

I know this isn't 100% the best example since it's literally 1 or 2 lines changed, but there are benefits for it somewhere...

[CodeCrafter47]

I object to shading another library just to get rid of a few IDE warnings. If we want to get rid of the beta warnings we can simply compile against a newer version of the guava library. Cache isn't in beta anymore since version 19 which has been release in 2015. BungeeCord currently ships version 21, so it would probably be okay to build against that. The danger with building against a newer version is that one might accidentally use a method not present in the older version, breaking the plugins compatibility with older versions of BungeeCord/ Spigot. That is why I prefer to build against the older version of the library. As for the get method, that is also available in guava, so you can actually make that change.

[Andre601]

I object to shading another library just to get rid of a few IDE warnings. If we want to get rid of the beta warnings we can simply compile against a newer version of the guava library. Cache isn't in beta anymore since version 19 which has been release in 2015. BungeeCord currently ships version 21, so it would probably be okay to build against that. The danger with building against a newer version is that one might accidentally use a method not present in the older version, breaking the plugins compatibility with older versions of BungeeCord/ Spigot. That is why I prefer to build against the older version of the library. As for the get method, that is also available in guava, so you can actually make that change.

I bumped the version to 21, but the get method seems to have a chance to throw an ExecutionException so I'm not sure what would be a good approach here. Just return null and handle that?

[CodeCrafter47]

The best approach would be to handle the exception by throwing another exception. The ExecutionException is thrown by the get method if the lambda parameter throws an exception. If I'm not mistaken, the code in the lambda doesn't throw any exceptions. So if we get a ExecutionException that indicates a bug in the code. I'd wrap it in an assertion error and throw it.

[CodeCrafter47]

Can this be merged, or do you still want to work on it?

[Andre601]

Unless you have any change requests?