-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
player/dialogue: Basic implementation of NPC dialogues #937
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like how this looks so far. Simple, clean, similar to existing APIs. Thumbs up for that! I've left some suggestions on how to update it to use transactions.
Additionally, I believe we discussed some things on Discord about how the entity should be shown in the dialogue, and how sending (altered) metadata before a dialogue to achieve the settings could solve it. Not sure if you wanted that in this PR too?
server/session/entity_metadata.go
Outdated
@@ -161,6 +161,9 @@ func (s *Session) addSpecificMetadata(e any, m protocol.EntityMetadata) { | |||
} | |||
m[protocol.EntityDataKeyVisibleMobEffects] = packedEffects | |||
} | |||
if d, ok := e.(dialogueProvider); ok { | |||
m[protocol.EntityDataKeyHasNPC] = boolByte(d.CanShowDialogue()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed before, I think it might be a good idea to just always set this to true and remove the dialogueProvider
interface, unless there's any real repercussions from that.
e798ed1
to
11f0fe1
Compare
Kind of a continuation from #666 however a lot of it is different.
Whilst this code works, we need a better method of implementing the
dialogueProvider
interface to allow entities such as players (often used for NPCs) to be a provider. We could just make it always implement the interface but I'm open to other ideas. There are also a few names I am not sure about, like theCanShowDialogue()
method to implement this interface.I have also decided to completely hide the idea of "opening" and "closing" buttons, they only exist in Minecraft as a way of detecting opening/closing of the menus which we can already achieve through calling
SendDialogue()
for opening and implementingdialogue.Closer
for closing.Example usage in combination with df-mc/npc where p is a
*player.Player
(modifying Player to implement dialogueProvider):