KyoriPowered / adventure

A user-interface library, formerly known as text, for Minecraft: Java Edition
https://docs.advntr.dev/
MIT License
679 stars 105 forks source link

`Formatter.date` resolver not working #952

Closed zefir-git closed 7 months ago

zefir-git commented 11 months ago

The documentation says inserting an instance of TemporalAccessor is possible using Formatter.date resolver. However, when using Date#toInstant() which returns Instant (which is TemporalAccessor) the replacement is not done.

Date date = new Date();
String message = "Date: <date:'yyyy-MM-dd HH:mm:ss'>";
sender.sendMessage(MiniMessage.miniMessage().deserialize(message, Formatter.date("date", Formatter.date("date", date.toInstant())));
// Message sent ↓
// Date: <date:'yyyy-MM-dd HH:mm:ss'>

No errors/warnings.

However, using the example from the docs (LocalDateTime.now(ZoneId.systemDefault())) works.

Joo200 commented 11 months ago

Instants doesn't contain a timezone information. You can use instant.atZone(ZoneId.systemDefault()) to get a TemporalAccessor with a timezone

zefir-git commented 11 months ago

@Joo200 I used instant.atZone(ZoneOffset.UTC).toLocalDateTime() and that worked. This might be good to mention in the docs as Instant is TemporalAccessor

Pablete1234 commented 11 months ago

The requirement here isn't necessarily zone information, but support for time fields that you're using in your own format. Instant only supports limited fields, while you're requesting to use a format that uses years, months, hours, etc. A ZonedDateTime, OffsetDateTime, or LocalDateTime should all support what you need here

In theory, even a LocalDate should be able to fill-in the formatting for yyyy-MM-dd just fine, but fail if you try to ask it for hour/min data, as that's something it cannot get.

I do believe it makes sene tho, that it throws some exception or log message when trying to use a temporal accesor that does not support all the fields you're requesting in the format