KyoriPowered / adventure

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

DateFormatter not clear about what object it needs #1003

Open CubBossa opened 9 months ago

CubBossa commented 9 months ago

The following test code fails, while both Instant.now() and LocalDateTime.now() implement TemporalAccessor and should theoretically work

        MiniMessage mm = MiniMessage.miniMessage();
        assertEquals(
                mm.deserialize("<date:'mm'>", Formatter.date("date", LocalDateTime.now())),
                mm.deserialize("<date:'mm'>", Formatter.date("date", Instant.now()))
        );
org.opentest4j.AssertionFailedError: 

Expected :TextComponentImpl{content="28", style=StyleImpl{obfuscated=not_set, bold=not_set, strikethrough=not_set, underlined=not_set, italic=not_set, color=null, clickEvent=null, hoverEvent=null, insertion=null, font=null}, children=[]}

Actual   :TextComponentImpl{content="<date:'mm'>", style=StyleImpl{obfuscated=not_set, bold=not_set, strikethrough=not_set, underlined=not_set, italic=not_set, color=null, clickEvent=null, hoverEvent=null, insertion=null, font=null}, children=[]}
Pablete1234 commented 9 months ago

This issue is a duplicate of an already closed one, see https://github.com/KyoriPowered/adventure/issues/952#issuecomment-1655951073

CubBossa commented 9 months ago

Alright thx, sorry for the duplicate. Maybe as requested in the other issue, there could be some accessible information on it, like producing an error or putting it into the docs of the method. Since Instant implements TemporalAccessor you would assume that it would work.

kezz commented 9 months ago

It generally boils down to just knowing that the that format you're writing is being backed by a date object that can provide the temporal information you would want. Perhaps we could be clearer in the JavaDocs, or throw a warning or something? I'll leave this issue open for that.