Inline Replies

[Neuheit]

Summary

This issue is for tracking developments related to inline replies. Right now it looks like it will be using message references as well as adding new data to message and activity types.

[Neuheit]

Currently waiting on discord/discord-api-docs#2118 to be merged.

[ProfDoof]

discord/discord-api-docs#2118 was merged.

[VelvetToroyashi]

How hard could it be :^)

[ProfDoof]

Famous last words.

Anyway, so to start a conversation about the expected changes.

1. Add the ReferencedMessage Property to the DiscordMessage. Should have type DiscordMessage
2. Update the XML for the MessageReference to reflect the changes in the way that it has been used
3. Add the new message type to the Enum MessageType
4. Add a ReplyAsync method to the DiscordMessage class (I'm going based on the fact that message_id is the only requirement to reply to a message according to the comment thread and this portion of the docs ).
5. Add a ReplyAsync method to the CommandContext class.

Since we don't have an AllowedMentions class as seen here, we should make a boolean on our ReplyAsync methods to specify whether to ping the user or not. Should we default to false on pinging on our side? Or should we leave the default behavior up to Discord?

This is all I can think of right now, and I might have missed some things. Let me know what y'all think.

[VelvetToroyashi]

Well at least on the client side, it defaults to mentioning.

I'd be more than happy to help when it comes to adding the DiscordMessage#ReplyAsync and CommandContext#ReplyAsync.

[VelvetToroyashi]

Also just gonna go on a hunch, given that the message reference only has a few Ids instead of a proper message object, I take it that property would only be a partial-object, and it's up to the end user to actually retrieve the message from REST if they want to get the full object? I don't know the deep inner-workings of the API, so that's my best guess :p

[ProfDoof]

From my (admittedly very basic) understanding referenced_message is a full message object. At least in V8. Not in V6. It doesn't even exist in V6.

[VelvetToroyashi]

Last I checked in the docs, it just sends the id of the message and author, but not full object. I'll take a glance again.

[ProfDoof]

There are two fields, message_reference (that one is what you are talking about) and referenced_message and that is the one I'm talking about.

[VelvetToroyashi]

Ohh yeah, that makes sense then. My bad.

[VelvetToroyashi]

https://discord.com/developers/docs/resources/channel#message-object-message-structure I see it here.

Also good grief those asterisks lmao

[Emzi0767]

Implementing this will require us to bump GW/API to v8.

[Neuheit]

Implementing this will require us to bump GW/API to v8.

Mason stated that this is still available for v6, but we'll just have to use an existing type rather than a new one.

[Neuheit]

Famous last words.

Anyway, so to start a conversation about the expected changes.

1. Add the ReferencedMessage Property to the DiscordMessage. Should have type DiscordMessage
2. Update the XML for the MessageReference to reflect the changes in the way that it has been used
3. Add the new message type to the Enum MessageType
4. Add a ReplyAsync method to the DiscordMessage class (I'm going based on the fact that message_id is the only requirement to reply to a message according to the comment thread and this portion of the docs ).
5. Add a ReplyAsync method to the CommandContext class.

Since we don't have an AllowedMentions class as seen here, we should make a boolean on our ReplyAsync methods to specify whether to ping the user or not. Should we default to false on pinging on our side? Or should we leave the default behavior up to Discord?

This is all I can think of right now, and I might have missed some things. Let me know what y'all think.

1. Yes
2. Yes
3. Yes (make sure it's type 0)
4. This can probably be added as an optional parameter to SendMessageAsync() instead. Aside from the view message history permission checking, make sure to follow the prerequisites at the bottom of create message. Make sure this is also added to the DiscordRestClient.
5. Similarly to above, it should be made an optional parameter in RespondAsync()

ChannelId for DiscordMessageReference will also need to be made nullable.
There will also need to be a RepliedUser property added to mentionables as per the comment you linked. The default behavior should be up to Discord for mentioning.

[Emzi0767]

Since this is an entirely new feature, adding new type vs reusing existing has little meaning here - either won't break the API surface. We have to migrate to v8 anyways, so better start now than later.

[Emzi0767]

Should also point out that the only real point of staying on v6 - optional intents - is entirely moot now.

[VelvetToroyashi]

Furthermore, I feel like adding more parameters to CommandContext#RespondAsync and DiscordChannel#SendMessageAsync is less than ideal, thought the latter would have to have an extra parameter since you need to pass the user. Meh. I digress.

Though I guess this is less important given that parameters can be made optional, but ¯\(ツ)/¯

[Neuheit]

Furthermore, I feel like adding more parameters to CommandContext#RespondAsync and DiscordChannel#SendMessageAsync is less than ideal, thought the latter would have to have an extra parameter since you need to pass the user. Meh. I digress.

Though I guess this is less important given that parameters can be made optional, but ¯_(ツ)_/¯

Optional parameters are ideal since SendMessageAsync is meant to mirror Discord's Create Message on the API docs.

[VelvetToroyashi]

Fair enough. Not like I've actually really done anything on the lib yet, so it's not exactly detrimental to just add it to the method instead of creating a whole new one.

[ProfDoof]

So, while I was attempting to understand the codebase well enough to actually start on adding the properties I noticed that we don't keep track of MessageReference when using the internal copy constructor. Do we need to add this at this time?

DSharpPlus/DSharpPlus/Entities/DiscordMessage.cs

Lines 45 to 71 in 0c04045

	internal DiscordMessage(DiscordMessage other)
	: this()
	{
	this.Discord = other.Discord;
	this._attachments = other._attachments; // the attachments cannot change, thus no need to copy and reallocate.
	this._embeds = new List<DiscordEmbed>(other._embeds);
	if (other._mentionedChannels != null)
	this._mentionedChannels = new List<DiscordChannel>(other._mentionedChannels);
	if (other._mentionedRoles != null)
	this._mentionedRoles = new List<DiscordRole>(other._mentionedRoles);
	this._mentionedUsers = new List<DiscordUser>(other._mentionedUsers);
	this._reactions = new List<DiscordReaction>(other._reactions);
	this._stickers = new List<DiscordMessageSticker>(other._stickers);
	this.Author = other.Author;
	this.ChannelId = other.ChannelId;
	this.Content = other.Content;
	this.EditedTimestampRaw = other.EditedTimestampRaw;
	this.Id = other.Id;
	this.IsTTS = other.IsTTS;
	this.MessageType = other.MessageType;
	this.Pinned = other.Pinned;
	this.TimestampRaw = other.TimestampRaw;
	this.WebhookId = other.WebhookId;
	}

[VelvetToroyashi]

Famous last words.
Anyway, so to start a conversation about the expected changes.

1. Add the ReferencedMessage Property to the DiscordMessage. Should have type DiscordMessage
2. Update the XML for the MessageReference to reflect the changes in the way that it has been used
3. Add the new message type to the Enum MessageType
4. Add a ReplyAsync method to the DiscordMessage class (I'm going based on the fact that message_id is the only requirement to reply to a message according to the comment thread and this portion of the docs ).
5. Add a ReplyAsync method to the CommandContext class.

Since we don't have an AllowedMentions class as seen here, we should make a boolean on our ReplyAsync methods to specify whether to ping the user or not. Should we default to false on pinging on our side? Or should we leave the default behavior up to Discord?
This is all I can think of right now, and I might have missed some things. Let me know what y'all think.

1. Yes
2. Yes
3. Yes (make sure it's type 0)
4. This can probably be added as an optional parameter to SendMessageAsync() instead. Aside from the view message history permission checking, make sure to follow the prerequisites at the bottom of create message. Make sure this is also added to the DiscordRestClient.
5. Similarly to above, it should be made an optional parameter in RespondAsync()

ChannelId for DiscordMessageReference will also need to be made nullable.
There will also need to be a RepliedUser property added to mentionables as per the comment you linked. The default behavior should be up to Discord for mentioning.

@fireflamemm brought up an interesting point. "Should the user pass the message Id, or should they pass the message object, and expect the library to handle it". Both make sense, since we can always grab the message Id from that message object, and then go about the normal reply flow.

[ProfDoof]

https://github.com/VelvetThePanda/DSharpPlus/blob/d1a49c77ac2f03efcda8ea2d2bb2469fe8736534/DSharpPlus/Clients/DiscordClient.Dispatch.cs#L1179-L1190

Ok, here's my current rough way of getting around propagating the update to the message that had been replied to by some user. However, I already see a problem and need other people with more experience to point me in the direction of a better solution. This solution will pass around a reference to a DiscordMessage that has content in it even if said DiscordMessage has been removed from the cache. This is going to prevent the GC from garbage collecting this message. This means that when someone has a DiscordMessage with a non-null ReferencedMessage they will always believe that this ReferencedMessage is non-null even if they delete the message that was referenced. This smells funny to me and I'd prefer a cleaner solution. However, the only other solution I can see right off the top of my head is to go through the cache looking for all messages that reference a message and forcibly updating their message.

[VelvetToroyashi]

Furthmore if anyone wants to take a glance at these files and give some feedback on overall quality of things. I was quite tired when I wrote this code, and it was cobbled together in <24Hr, so, not my proudest. Thanks to Wam for helping out on it, too.

ApiClient.cs
RestChannelPayloads.cs

DiscordRestClient.cs

CommandContext.cs

Looking at that last one reminded me I have to add replies to uploading files, but I'll get to that within the next day or two, if nobody else wants to :p