Add first and last played to /seen output

[Bobcat00]

Information

This PR closes #4572 .

Details

Proposed feature:

This PR adds the online/offline player getFirstPlayed() and getLastPlayed() to the output of the /seen command. This can be useful to see if someone is a long-time player on the server.

Environments tested:

OS: Windows 10

Java version: Java(TM) SE Runtime Environment (build 16.0.2+7-67)

• Most recent Paper version (1.17.1, git-Paper-372)
• CraftBukkit/Spigot/Paper 1.12.2
• CraftBukkit 1.8.8

Demonstration:

I could add a permissions check if you'd like (I'd probably choose essentials.seen.dates unless you have some other preference).

While I'm in here, I could also fix up the UUID and history order which is half done as part of another PR.

[Bobcat00]

P.S. This is on all the time on my server, and my players really like it. That's why I didn't include a permissions check.

[pop4959]

A similar PR has been proposed before, I would recommend looking through the comments on #4265 for context.

[oskarkk]

If many people want that small simple feature, why don't just add it and allow users to choose if they want to use it or not? There's no other way to get this information about an offline player.

[pop4959]

If many people want that small simple feature, why don't just add it and allow users to choose if they want to use it or not? There's no other way to get this information about an offline player.

There were some opinions in the previous PR as to whether this belongs in /seen, /whois, or both, but I don't think we ever really settled in on it because the author closed the PR before we finished discussing that. Personally it sounds like this would probably fit better in /seen since you could get this information regardless of whether the player is online.

[Bobcat00]

This definitely belongs in /seen, not /whois. If you want to make it optional, I can add a permission.

My PR uses the Java predefined date and time styles based on the locale of the server. So if you're in France, it will show 6 novembre 2021, 17:49.

I've had this feature on my server for 7 years! It's really convenient. When I had to temporarily remove my plugin due to a Minecraft update, my players were asking me what happened to the join / last played dates.

[pop4959]

My personal opinion is that a permission isn't necessary here, since I don't see why anyone would want to hide this like they would for UUID or GeoIP.

For the date format I think we still want to consider how we could expose this to translators and make it consistent with the format used in other parts of the plugin (notably mail). Automatically setting the style based on the locale of the server means that it will ignore changes to the language setting in the Essentials configuration, which obviously isn't the best.

[Bobcat00]

I can specify the locale when calling DateFormat#getDateTimeInstance.

public static final DateFormat getDateTimeInstance(int dateStyle,
                                                   int timeStyle,
                                                   Locale aLocale)

But if you want it to be the same as the ugly timestamp for mail, we can make this ugly, too.
We don't write dates as [2021/11/06 18:28] here.

[Bobcat00]

Correct me if I'm wrong, but mail has the date and time format hardcoded. How is that better? And it doesn't account for the Essentials language setting or the locale.

Essentials/Essentials/src/main/java/com/earth2me/essentials/MailServiceImpl.java

Line 16 in 9c45127

private final transient ThreadLocal<SimpleDateFormat> df = ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy/MM/dd HH:mm"));

Java lets you construct a Locale based on a language string and, optionally, a country string.

[pop4959]

I can specify the locale when calling DateFormat#getDateTimeInstance.
But if you want it to be the same as the ugly timestamp for mail, we can make this ugly, too.
We don't write dates as [2021/11/06 18:28] here.

I'm not suggesting one format over the other. As far as I know, before the mail revamp, there were no such date formats displayed in Essentials as everything only used datediff.

Correct me if I'm wrong, but mail has the date and time format hardcoded. How is that better? And it doesn't account for the Essentials language setting or the locale.

It's not better, which is why I have been saying that this needs to be exposed to translators, regardless of what default one might think looks "good" or "ugly" (which is completely subjective). Updating the mail format is outside the scope of this PR but it should definitely also be updated.

[Bobcat00]

I can do some experimenting with Java's Locale and the Essentials' setting. But of course, I won't know if Japanese or Thai is correct.

It looks like Java's processing of locales has changed over the years. What version of Java is used to compile Essentials?

If I were to make a Locale object based on the config setting that different parts of Essentials can use, where should that object be created?

[pop4959]

Thanks for taking initiative. Essentials still supports Java 8 since older versions of Minecraft cannot be run on newer Java versions. For the locale it looks like you should already be able to get this using I18n#getCurrentLocale.

[Bobcat00]

For the locale it looks like you should already be able to get this using I18n#getCurrentLocale.

Cool! That should make it easier. I'll work on this more tomorrow.

[Bobcat00]

This PR now uses the Essentials' locale defined in its config file. Here are examples using en (default), en_GB (note change in date format from Month Day Year to Day Month Year and use of 24 hour time), de, fr, and ru. Only change between each example was changing the config's locale and doing ess reload.

[Bobcat00]

Hey guys, I finished this PR two weeks ago. Any additional comments?

[mdcfe]

I'm not particularly keen on adding even more info lines to /seen - the command's output has gotten increasingly cluttered over the past few months. If we add this, it needs to be configurable through perms and off by default.

[Bobcat00]

Alright.