Add Inline Replies in response to #676 #704

VelvetToroyashi · 2020-11-29T22:41:46Z

Summary

Addresses #676. Adds Inline Replies to messages.

Details

In order to address recent changes to the Discord API which added inline replies this PR is to add replies for use by the client. This also adds a way to parse the payloads from Discord which contain ReferencedMessage which is the official way to get messages in V8. In V6 this requires using the Reference to get the message. This PR also adds the message replied to into the cache if it isn't already there.

Changes proposed

Add the Message ID parameter to RespondAsync on both the DiscordMessage and the CommandContext as the message to reply to.
Add the Message ID parameter to CreateMessageAsync as the message to reply to.
Add the Message ID parameter to SendMessageAsync on the DiscordClient and the DiscordChannel as the message to reply to.
Add the Mention parameter to SendMessageAsync and RespondAsync as to whether a reply should ping the user replied to or not.
Add test commands to the test bot to test replying to messages
[TODO] Add test commands to test that ReferenceMessages are parsed correctly.
Extract methods from the OnMessageCreateEventAsync and OnMessageUpdateEventAsync that are used for both events and to populate the final fields in the ReferenceMessage
Add Mention Property to DiscordMentions object
Add ReferencedMessage property to the DiscordMessage object in prep for the move to V8.
Make ChannelId nullable in the DiscordMessageReference
Add RepliedUserMention struct
Add new Reply message type to MessageType enum in preparation for the move to V8
Add MessageReference to RestChannelMessageCreatePayload
Extract methods from PrepareMessage in DiscordApiClient to use on both the referenced message and the message.

Notes

N/A

- Add ReferencedMessage Property to DiscordMessage - Extracted Method Populating DiscordMessage in OnMessageCreateEventAsync - Populated the ReferencedMessage Property when it wasn't null in DiscordMessage.

…cessary files, but the reference is serialized no matter what, making it impossible to actually send regular messages.

… bit unclean. I'll clean it up hopefully

…great, but it's something.

…replies without having to duplicate a class. Also removes the need for casting. Thanks, Wam <3 I'm tired lmao

… which defaults to false, as per the API.

…Reference, as it previously attempted to send a null channel id otherwise, causing the API to treat it as a reply, throwing a 400 in response.

…into master

- Fix nit in naming which used underline case instead of camel case - Allow for reference messages to update their user and member as well - Add Reply MessageType to MessageType enum - Extract message population from PrepareMessage to a helper method which can then be used for both the Message and the ReferencedMessage

sh30801 · 2020-12-03T14:52:55Z

DSharpPlus.CommandsNext/EventArgs/CommandContext.cs

-        public Task<DiscordMessage> RespondAsync(string content = null, bool isTTS = false, DiscordEmbed embed = null, IEnumerable<IMention> mentions = null) 
-            => this.Message.RespondAsync(content, isTTS, embed, mentions);
+        public Task<DiscordMessage> RespondAsync(string content = null, bool isTTS = false, DiscordEmbed embed = null, 
+            IEnumerable<IMention> mentions = null, ulong? message_id = null, bool mention = false) 


I know I said to include a message id rather than a full message object, but looking back I am not sure that was a good idea on my part. I think this should be changed to a DiscordMessage object, but any opinions against this are welcome.

I run into a lot of cases where I have a message ID from somewhere else (eg. a database lookup) and don't have a full DiscordMessage object on hand. At the moment it seems the only way to create a DiscordMessage object is to get it through the REST client, which would make an unnecessary API call just to get a wrapped object.

(I would also welcome ways to create the model objects from other data sources but that's way out of scope for this PR.)

You're able to get it from the DiscordClient's message cache though right? The main reason I am against just using IDs is due to consistency because most of our other methods require a full object. IDs would also still be able to be used with the DiscordRestClient.

Not if it's a message that's not in the message cache. I turn the message cache off entirely in my bot because it's more or less useless at scale, and it frequently deals with messages that are old enough to be long out of the cache anyway.

Fair enough. We'll have to rework how we handle passing complete objects to REST, but that would be out of scope for this PR.

DSharpPlus/Clients/DiscordClient.Dispatch.cs

DSharpPlus.CommandsNext/EventArgs/CommandContext.cs

DSharpPlus/Clients/DiscordClient.Dispatch.cs

DSharpPlus/Net/DiscordApiClient.cs

DSharpPlus/Entities/DiscordMessage.cs

DSharpPlus/Entities/DiscordMentions.cs

DSharpPlus/Clients/DiscordClient.Dispatch.cs

ProfDoof · 2020-12-03T16:11:59Z

DSharpPlus/Clients/DiscordClient.cs

+        public Task<DiscordMessage> SendMessageAsync(DiscordChannel channel, string content = null, bool isTTS = false, 
+                DiscordEmbed embed = null, IEnumerable<IMention> mentions = null, ulong? replyMessageId = null, bool mention = false)
+            => this.ApiClient.CreateMessageAsync(channel.Id, content, isTTS, embed, mentions, replyMessageId);


You didn't use the mention bool here.

ProfDoof · 2020-12-03T16:13:20Z

DSharpPlus/Entities/DiscordChannel.cs

        /// <exception cref="Exceptions.BadRequestException">Thrown when an invalid parameter was provided.</exception>
        /// <exception cref="Exceptions.ServerErrorException">Thrown when Discord is unable to process the request.</exception>
-        public Task<DiscordMessage> SendMessageAsync(string content = null, bool tts = false, DiscordEmbed embed = null, IEnumerable<IMention> mentions = null)
+        public Task<DiscordMessage> SendMessageAsync(string content = null, bool tts = false, DiscordEmbed embed = null, IEnumerable<IMention> mentions = null, ulong? message_id = null, bool mention = false)


Nit: the name doesn't match the conventions used elsewhere in the file

I'm pretty sure this is the case in other places I just didn't look long enough to find all of them.

DSharpPlus/Entities/DiscordMentions.cs

DSharpPlus/Enums/MessageType.cs

Emzi0767 · 2020-12-08T16:58:53Z

This is a chonker, so I'll be reviewing this one around Christmas, I'd like a summary of answers to the following questions:

Is this implementation v8-compatible (see Tracking: API and Gateway v8 #681)?
Instead of arguments to send message methods, could you implement this in terms of Adds a Message Builder #695 (making this PR dependent on that one)?

…uest. Creating a message reference will return null if the channel's Id is null Added replies to uploading files

Co-authored-by: Prof Doofenshmirtz <jmm15f@acu.edu>

VelvetToroyashi · 2020-12-20T16:40:07Z

DSharpPlus/Enums/MessageType.cs

        /// Message when a user replies to another user
        /// </summary>
-        Reply = 19,
+        Reply = 19


Did I really forget about this? 🤦🏽‍♂️

Wait I didn't even author this. I'm blind

sh30801 · 2020-12-20T17:00:15Z

DSharpPlus/Entities/DiscordMessage.cs

                };
-
-            var channel = client.InternalGetCachedChannel(channelId);
+            if (channelId.HasValue) return null;


This check doesn't make sense, and a message reference can still be built with a message id.

Oh wait is the channel Id nullable? I guess I didn't check, or misread what you said when I asked when about how to go about it

channel_id is optional when creating a reply, but will always be present when receiving an event/response that includes this data model.

If it's null, assume it's the message's own channel ID.

…ge reference

Kiritsu · 2021-01-04T22:21:54Z

DSharpPlus/Entities/DiscordMessageBuilder.cs

+        /// <summary>
+        /// Gets the Reply Message ID.
+        /// </summary>
+        public ulong? ReplyID { get; private set; } = null;


this has been completed

Kiritsu · 2021-01-04T22:22:20Z

DSharpPlus/Entities/DiscordMessageBuilder.cs

+        /// <param name="messageID">The ID of the message to reply to.</param>
+        /// <param name="mention">If we should mention the user in the reply.</param>
+        /// <returns></returns>
+        public DiscordMessageBuilder WithReply(ulong messageID, bool mention = false)


messageId

this has been completed

jeffreyladd · 2021-01-04T22:24:13Z

I have brought everything up to date with master and also added in the Inline replies to the Message Builder. Everything has been tested although am seeing an error coming from CommandNext but no stacktrace

Need to drill and see if its something that this branch did or something that snuck through in one of the other commits. Pinging @Neuheit have u noticed this?

sh30801 · 2021-01-12T15:51:13Z

I have brought everything up to date with master and also added in the Inline replies to the Message Builder. Everything has been tested although am seeing an error coming from CommandNext but no stacktrace

Need to drill and see if its something that this branch did or something that snuck through in one of the other commits. Pinging @Neuheit have u noticed this?

No I haven't, does it still work with that exception thrown?

VelvetToroyashi · 2021-01-12T15:53:58Z

Does it not print that exception to the console? Looks like the debugger output to me (it's showing threads starting and stopping), and even in Rider the debugger output simply says an exception happens, but the stack trace is in the console

- Always provide mentions, defaulting to an empty array - Role mentions go based off `mention_roles` supplied by Discord - Message builder now supplies a `RepliedUserMention` if set to mention, which may or may not be necessary. - Added a test command to validate that mentions are preserved and `Mentioned______` only returns mentions that discord identifies (that is, mentions that "ping"). This should address a long-time bug introduced by DSharpPlus#704, which was not fixed in 932, nor identified in DSharpPlus#837.

* Fix reply mention oddities * Leave it to me to almost ship a faulty PR * Fix replies with other people * Fix role mentions * Apply accessor and add space * Fix replies - Always provide mentions, defaulting to an empty array - Role mentions go based off `mention_roles` supplied by Discord - Message builder now supplies a `RepliedUserMention` if set to mention, which may or may not be necessary. - Added a test command to validate that mentions are preserved and `Mentioned______` only returns mentions that discord identifies (that is, mentions that "ping"). This should address a long-time bug introduced by #704, which was not fixed in 932, nor identified in #837. * Revert to Mentions.All and merge origin into local

@Kiritsu

* Add ReferencedMessage's to DiscordMessage - Add ReferencedMessage Property to DiscordMessage - Extracted Method Populating DiscordMessage in OnMessageCreateEventAsync - Populated the ReferencedMessage Property when it wasn't null in DiscordMessage. * Added `message_reference` to `RestChannelPayloads.cs`, and updated necessary files, but the reference is serialized no matter what, making it impossible to actually send regular messages. * Got replies to work by casting to the appropriate type, though it's a bit unclean. I'll clean it up hopefully * Just wanted to make sure I committed what I currently have. It ain't great, but it's something. * Made `InternalDiscordMessageReference#ChannelId` nullable to support replies without having to duplicate a class. Also removes the need for casting. Thanks, Wam <3 I'm tired lmao * Fixed a param not being nullable ulong, and added toggleable mention, which defaults to false, as per the API. * add `NullValueHandling.Ignore` to channelId in InternalDiscordMessageReference, as it previously attempted to send a null channel id otherwise, causing the API to treat it as a reply, throwing a 400 in response. * Add way for referenced messages to be referenced from cache * Add Reference Messages to GetMessageAsync and GetMessages - Fix nit in naming which used underline case instead of camel case - Allow for reference messages to update their user and member as well - Add Reply MessageType to MessageType enum - Extract message population from PrepareMessage to a helper method which can then be used for both the Message and the ReferencedMessage * Correct "this" usage and null checks * Move Populate methods to the DiscordClient class * Changed the message reference creation behavior as per @Neuheit's request. Creating a message reference will return null if the channel's Id is null Added replies to uploading files * Changed the message reference creation behavior as per @Neuheit's request. Creating a message reference will return null if the channel's Id is null Added replies to uploading files * Update DSharpPlus/Enums/MessageType.cs Co-authored-by: Prof Doofenshmirtz <jmm15f@acu.edu> * Removed value check on channelId when constructing a new DiscordMessage reference * Reapply Replies with new Message Builder * Tweak message_create and message_update events to not blow up * Suggestion made by @Kiritsu * Fixed excessive whitespace. Co-authored-by: jmmarsde <jmmarsde@ncsu.edu> Co-authored-by: Prof Doofenshmirtz <jmm15f@acu.edu> Co-authored-by: fireflamemm <johnswork15@gmail.com> Co-authored-by: Neuheit <38368299+Neuheit@users.noreply.github.com> Co-authored-by: Jeffrey Ladd <jladd@Jeffreyladd.net>

* Fix reply mention oddities * Leave it to me to almost ship a faulty PR * Fix replies with other people * Fix role mentions * Apply accessor and add space * Fix replies - Always provide mentions, defaulting to an empty array - Role mentions go based off `mention_roles` supplied by Discord - Message builder now supplies a `RepliedUserMention` if set to mention, which may or may not be necessary. - Added a test command to validate that mentions are preserved and `Mentioned______` only returns mentions that discord identifies (that is, mentions that "ping"). This should address a long-time bug introduced by #704, which was not fixed in 932, nor identified in #837. * Revert to Mentions.All and merge origin into local

ProfDoof and others added 10 commits November 25, 2020 02:29

Add ReferencedMessage's to DiscordMessage

5ea8239

- Add ReferencedMessage Property to DiscordMessage - Extracted Method Populating DiscordMessage in OnMessageCreateEventAsync - Populated the ReferencedMessage Property when it wasn't null in DiscordMessage.

Added message_reference to RestChannelPayloads.cs, and updated ne…

c5288a2

…cessary files, but the reference is serialized no matter what, making it impossible to actually send regular messages.

Got replies to work by casting to the appropriate type, though it's a…

a117f8c

… bit unclean. I'll clean it up hopefully

Just wanted to make sure I committed what I currently have. It ain't …

1bcf895

…great, but it's something.

Made InternalDiscordMessageReference#ChannelId nullable to support …

3f9f9ef

…replies without having to duplicate a class. Also removes the need for casting. Thanks, Wam <3 I'm tired lmao

Fixed a param not being nullable ulong, and added toggleable mention,…

667f42d

… which defaults to false, as per the API.

add NullValueHandling.Ignore to channelId in InternalDiscordMessage…

e274162

…Reference, as it previously attempted to send a null channel id otherwise, causing the API to treat it as a reply, throwing a 400 in response.

Add way for referenced messages to be referenced from cache

333c083

Merge branch 'master' of https://github.com/VelvetThePanda/DSharpPlus …

d1a49c7

…into master

Giggitybyte added core new-features v4.x labels Nov 29, 2020

sh30801 linked an issue Nov 30, 2020 that may be closed by this pull request

Inline Replies #676

Closed

sh30801 reviewed Dec 3, 2020

View reviewed changes

Correct "this" usage and null checks

dee3d30

ProfDoof reviewed Dec 3, 2020

View reviewed changes

Move Populate methods to the DiscordClient class

5418984

VelvetToroyashi added 2 commits December 13, 2020 15:41

Changed the message reference creation behavior as per @Neuheit's req…

8e84cf8

…uest. Creating a message reference will return null if the channel's Id is null Added replies to uploading files

Changed the message reference creation behavior as per @Neuheit's req…

de4a643

…uest. Creating a message reference will return null if the channel's Id is null Added replies to uploading files

VelvetToroyashi requested a review from sh30801 December 13, 2020 20:43

Kiritsu mentioned this pull request Dec 20, 2020

Replied user mention is not included in Message.MentionedUsers #717

Closed

Update DSharpPlus/Enums/MessageType.cs

aff9933

Co-authored-by: Prof Doofenshmirtz <jmm15f@acu.edu>

VelvetToroyashi commented Dec 20, 2020

View reviewed changes

sh30801 reviewed Dec 20, 2020

View reviewed changes

VelvetToroyashi and others added 4 commits December 20, 2020 12:08

Removed value check on channelId when constructing a new DiscordMessa…

fa41b90

…ge reference

Merge remote-tracking branch 'origin/master'

5da5171

Merge from master

6259b98

Reapply Replies with new Message Builder

a0eebb7

Tweak message_create and message_update events to not blow up

56f59a5

Kiritsu suggested changes Jan 4, 2021

View reviewed changes

Suggestion made by @Kiritsu

7f1cc6a

Kiritsu approved these changes Jan 4, 2021

View reviewed changes

jeffreyladd approved these changes Jan 4, 2021

View reviewed changes

Merge branch 'master' into master

c89eba4

Fixed excessive whitespace.

26549e3

Emzi0767 merged commit 94e6c45 into DSharpPlus:master Jan 13, 2021

VelvetToroyashi mentioned this pull request Jan 14, 2021

Replies do not mention when toggled to. #731

Closed

Emzi0767 added this to the v4.0 release milestone Jan 14, 2021

VelvetToroyashi mentioned this pull request Jun 15, 2021

Fix inline replies breaking mentions #932

Merged

Uh oh!

Add Inline Replies in response to #676 #704

Add Inline Replies in response to #676 #704

Uh oh!

Conversation

VelvetToroyashi commented Nov 29, 2020

Summary

Details

Changes proposed

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xSke Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProfDoof Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Emzi0767 commented Dec 8, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffreyladd commented Jan 4, 2021

Uh oh!

sh30801 commented Jan 12, 2021

Uh oh!

VelvetToroyashi commented Jan 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

xSke Dec 7, 2020 •

edited

Loading

ProfDoof Dec 3, 2020 •

edited

Loading