Add getOfflinePlayerIfCached(String)

[oxygencraft]

Closes #4264

[electronicboy]

IMHO, the deprecations are not needed, they were added by CB to discourage the usage of strings for looking up players, the biggest caveat is around the potential lookups that the standard getOfflinePlayer will induce against Mojang's API, this is not an issue here, and there are perfectly valid cases for looking up by a name, e.g. user commands

These deprecations have mostly served their course, only real caveat which I err against removing the one on getOfflinePlayer(String) is due to that "hidden" potential lookup hit, but that's not an issue here, and I'm not really sure it makes sense to expect every plugin that works on offline players to have their own cache when the server already has one, which is now being exposed

open to comments, but, I think that the deprecations have mostly run their course, and the only real reason for trying to shove people off them has past

[oxygencraft]

OK, I will remove the deprecation tag from the method. The reason why I added it is because I didn't know anything you just said at the time and I thought that because getOfflinePlayer(String) was deprecate, I would need to deprecate my method too.

[Shevchik]

Tbh, i would probably argue that such method should be called as getLocalOfflinePlayer and should state that this method will return an offline/online player which played locally on a server at least once, otherwise null (won't be the same as current definition tho).

The reasoning behind this is that return if cached/not doing a web request is way too abstract definition for that.
It is not so simple to understand which cache are we talking about because 2 caches exist: player name->uuid cache, bukkit all offline players cache. Same thing with web requests, there are many ways how bukkit impl can implement getting offline player that never played on a server, without doing a web request.

Also current implementation uses UserCache, which may be entirely disabled, and probably won't work in offline mode.

[BillyGalbreath]

Tbh, i would probably argue that such method should be called as getLocalOfflinePlayer and should state that this method will return an offline/online player which played locally on a server at least once, otherwise null (won't be the same as current definition tho).

The reasoning behind this is that return if cached/not doing a web request is way too abstract definition for that.

I would agree, except the implementation is literally checking against usercache.json which entries do expire from, does get truncated by entry size, can be disabled, and can be altered. So to say it returns an offline player that has played at least once is not factual. You point of making sure this method name represents clearly whats under the hood just verifies the current naming is indeed the correct choice ;)

[Proximyst]

@limbo-app ready

[limbo-app]

Pull request automatically approved for merging - a core team member has previously approved this pull request.