Allow components in conversation prompts

[kumpelblase2]

This allows prompts to alternatively return a text component, implementing #6644.

I'm not entirely happy with how it is right now, but the alternative I've explored also didn't sit well with me. Specifically, the Prompt now doesn't force a user to implement a method of getting something to display. Both methods are default, so there's no hard requirement and it will just compile fine without overwriting either of them and eventually crash at runtime. Unfortunately, making either of them non-default would be unsatisfactory in my eyes.

• Making getPromptMessage (the new API) non-default: Breaks every prompt in existence, so not an option.
• Making getPromptText (the existing API) non-default: Any prompts that want to return a component directly now have to deal with a dead method.

The alternative that I explored was creating a new prompt interface, say ComponentPrompt that would include the new method and provide a default for the getPromptText method.

ComponentPrompt

public interface ComponentPrompt extends Prompt {

  @Override
  @NotNull
  default String getPromptText(@NotNull ConversationContext context) {
      throw new UnsupportedOperationException("Use getPromptMessage() for component prompts.");
  }

  /**
   * Gets the message component to display to the user when this prompt is first
   * presented.
   *
   * @param context Context information about the conversation.
   * @return The chat component to display.
   */
  @NotNull Component getPromptMessage(ConversationContext context);
}

This would ensure old prompts won't break as they wouldn't be forced to implement this new interface. It would also force users of both prompt types (the old and new) to implement the correct method for getting the prompt text. I think this was more satisfactory from a user perspective, but this would now require instance checking internally when using the prompts. As a result I found this solution also a little unsatisfactory. But maybe the upside of having a clear guide on what to implement is more important?

Unfortunately, adding sendMessage(Component) to the Conversible causes a clash with the equivalent method in Audience - to some degree on purpose. Since it was defined as default there, it had to be overwritten in both Player and ConsoleCommandSender. Not particularly happy about this either, but creating yet another new method for this kind of thing felt awful. Overriding with the default implementation felt like a lesser evil.

I was also debating allowing the prefix to be a component too while I was at it. It would effectively have the same implementation challenge as the prompt so decided to leave that aside for now and see where we're going with the prompt implementation.

I acknowledge that this is a small and low priority change but I'd still be appreciative of feedback.

[MiniDigger]

I don't have an IDE right now, nor do I have the whole class hierarchy in my mind, but couldn't you avoid the issue with sendMessage by making conversable an audience, just like command sender? then the clash goes avoid, right?

generally, for adventure stuff, we omitted the get prefix from the getters, I guess this should apply here too?

[MiniDigger]

retried the pipeline because it got stuck, now you got failed tests https://github.com/PaperMC/Paper/pull/6726/checks?check_run_id=3814340093#step:5:1729

[kumpelblase2]

I considered that option, but I did not go that way because I didn't think it would make much sense to have functionality like showBossBar, showTitle, etc. be defined for a Conversable. It would work around that issue, that is correct. I guess it's not much of an issue given that everything has a default implementation and both console command sender & player already implement audience anyway.

generally, for adventure stuff, we omitted the get prefix from the getters, I guess this should apply here too?

Alright, will update accordingly.

[MiniDigger]

oh right, those are defined on audience too, yeah, fair enough then

[jpenilla]

I considered that option, but I did not go that way because I didn't think it would make much sense to have functionality like showBossBar, showTitle, etc. be defined for a Conversable. It would work around that issue, that is correct. I guess it's not much of an issue given that everything has a default implementation and both console command sender & player already implement audience anyway.

generally, for adventure stuff, we omitted the get prefix from the getters, I guess this should apply here too?

Alright, will update accordingly.

I think extending Audience is a better solution, all Conversables are also Audiences as is, and I don't see that changing really.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. If the issue still applies to the most recent supported version, please open a new issue referencing this original issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. If the issue still applies to the most recent supported version, please open a new issue referencing this original issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. If the issue still applies to the most recent supported version, please open a new issue referencing this original issue.

[Owen1212055]

What do we even want to do with the conversation api? I’m honestly surprised people even use it.

Is this something we want to add onto and upkeep? Or do we just want to push this api to the side here?

[olijeffers0n]

I used it once, it was honestly kind of a pain to use and would probably be easier to implement myself just using chat events. I would be happy to just deprecate and move on from it.

[Owen1212055]

Seeing as though there hasn't been really much push for this, and seeing that this API generally hasn't aged well and is most likely better being phased out this will be closed.

This API looks to be full of legacy string usage in general, and although this is kind of a workaround in order to allow modern components it's probably best to just encourage people to move away from this entirely.

Thank you for your contribution, regardless. 😄