Skip to content
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.

Sign up for GitHub

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

Jump to bottom

File forwarding #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

File forwarding #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
942c6cb
Attempt at adding support for direct files (plus multilple files from…
Mar 19, 2021
a89ee91
fixed backwards ternaries for multi-image force-Embed scenarios
Mar 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions Voltaire/Controllers/Messages/Send.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
using Rijndael256;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Text;
using System.Threading.Tasks;

namespace Voltaire.Controllers.Messages
{
class Send
{
private static HttpClient client = new HttpClient();

public static async Task PerformAsync(ShardedCommandContext context, string channelName, string message, bool reply, DataBase db)
{
var candidateGuilds = GuildList(context);
Expand All @@ -31,32 +35,56 @@ public static async Task PerformAsync(ShardedCommandContext context, string chan
}
}

public static Func<string, string, Task<IUserMessage>> SendMessageToChannel(IMessageChannel channel, bool replyable, ShardedCommandContext context, bool forceEmbed = false)
public static Func<string, string, Task<IUserMessage>> SendMessageToChannel(IMessageChannel channel, bool replyable, ShardedCommandContext context, bool forceEmbed = false, int attachmentIndex = -1)
{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(to continue my previous comment on shifting the check into this function)

If we can do the logic here - I think the goal would be to abstract the check for the existence of files to the top of this function, and leave the branching below agnostic. You may need to add another layer of abstraction, but the branching below shouldn't need to occur in multiple places.

Copy link
Author

@DJFarr DJFarr Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is, there are 4 conditions that change the text that is passed along into the message. Adding the attachment check makes a total of 8 options.
Instead of doing the attachments check before this and branching before the text has been finalised, it might be better to use the 4 branches to generate the text and then pass along the resulting message text into another function, which can then determine what to do about attachments, with the text just being 1 input argument. This function would return a Task<IUserMessage> back to the Send.SendMessageToChannel

Copy link
Owner

@nminchow nminchow Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - I'm fine with either option. I think it would need to do something like that, in fact. Some way to capture which message function should be used. This could be another function that is called which wraps SendMessageAsync, or a function which is always called by the returned function.

if (!replyable)
{
return async (username, message) =>
{
Stream file = attachmentIndex < 0 ? null : await client.GetStreamAsync(context.Message.Attachments.ElementAt(attachmentIndex).Url);

message = CheckForMentions(channel, message);
if (forceEmbed)
{
var view = Views.Message.Response(username, message, null);
return await SendMessageAndCatchError(() => { return channel.SendMessageAsync(view.Item1, embed: view.Item2); }, context);
return await SendMessageAndCatchError(() =>
{ //TODO SendFileAsync DONE
return attachmentIndex < 0 ?
channel.SendMessageAsync(view.Item1, embed: view.Item2) :
channel.SendFileAsync(file, context.Message.Attachments.ElementAt(attachmentIndex).Filename, view.Item1, embed: attachmentIndex == 0 ? view.Item2 : null);
}, context);
}

if (string.IsNullOrEmpty(username))
{
return await SendMessageAndCatchError(() => { return channel.SendMessageAsync(message); }, context);
return await SendMessageAndCatchError(() =>
{ //TODO SendFileAsync DONE
return attachmentIndex < 0 ?
channel.SendMessageAsync(message) :
channel.SendFileAsync(file, context.Message.Attachments.ElementAt(attachmentIndex).Filename, attachmentIndex == 0 ? message : "");
}, context);
}
return await SendMessageAndCatchError(() => { return channel.SendMessageAsync($"**{username}**: {message}"); }, context);
return await SendMessageAndCatchError(() =>
{ //TODO SendFileAsync DONE
return attachmentIndex < 0 ?
channel.SendMessageAsync($"**{username}**: {message}") :
channel.SendFileAsync(file, context.Message.Attachments.ElementAt(attachmentIndex).Filename, attachmentIndex == 0 ? $"**{username}**: {message}" : "");
}, context);
};
}
return async (username, message) =>
{
Stream file = attachmentIndex < 0 ? null : await client.GetStreamAsync(context.Message.Attachments.ElementAt(attachmentIndex).Url);

var key = LoadConfig.Instance.config["encryptionKey"];
var replyHash = Rijndael.Encrypt(context.User.Id.ToString(), key, KeySize.Aes256);
var view = Views.Message.Response(username, message, replyHash.ToString());
return await SendMessageAndCatchError(() => { return channel.SendMessageAsync(view.Item1, embed: view.Item2); }, context);
return await SendMessageAndCatchError(() =>
{ //TODO SendFileAsync DONE
return attachmentIndex < 0 ?
channel.SendMessageAsync(view.Item1, embed: view.Item2) :
channel.SendFileAsync(file, context.Message.Attachments.ElementAt(attachmentIndex).Filename, view.Item1, embed: attachmentIndex == 0 ? view.Item2 : null);
}, context);
};
}

Expand Down
18 changes: 15 additions & 3 deletions Voltaire/Controllers/Messages/SendDirectMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,21 @@ public static async Task PerformAsync(ShardedCommandContext context, string user

var userChannel = await userGuild.Item1.GetOrCreateDMChannelAsync();
var prefix = PrefixHelper.ComputePrefix(context, userGuild.Item2, "anonymous user");
var messageFunction = Send.SendMessageToChannel(userChannel, replyable, context);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
if (context.Message.Attachments.Any())
{
for (int attachmentIndex = 0; attachmentIndex < context.Message.Attachments.Count; attachmentIndex++)
{
var messageFunction = Send.SendMessageToChannel(userChannel, replyable, context, false, attachmentIndex);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
}
}
else
{
var messageFunction = Send.SendMessageToChannel(userChannel, replyable, context);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
}
await Send.SendSentEmote(context);
}
catch (Exception ex)
Expand Down
20 changes: 16 additions & 4 deletions Voltaire/Controllers/Messages/SendReply.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static async Task PerformAsync(ShardedCommandContext context, string repl
// TODO: potentially want to bake guilds into reply codes so we can ensure that the the replier isn't banned on the server where the original
// message was sent
var users = SendDirectMessage.ToUserList(candidateGuilds).Where(x => x.Id.ToString() == candidateId);
if(users.Count() == 0)
if (users.Count() == 0)
{
await Send.SendErrorWithDeleteReaction(context, "Something is wrong with that reply code. It is possible the sender has left your server.");
return;
Expand All @@ -36,9 +36,21 @@ public static async Task PerformAsync(ShardedCommandContext context, string repl

// all 'users' here are technically the same user, so just take the first
var channel = await users.First().GetOrCreateDMChannelAsync();
var messageFunction = Send.SendMessageToChannel(channel, replyable, context);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
if (context.Message.Attachments.Any())
{
for (int attachmentIndex = 0; attachmentIndex < context.Message.Attachments.Count; attachmentIndex++)
{
var messageFunction = Send.SendMessageToChannel(channel, replyable, context, false, attachmentIndex);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
}
}
else
{
var messageFunction = Send.SendMessageToChannel(channel, replyable, context);
var sentMessage = await messageFunction(prefix, message);
await Send.AddReactionToMessage(sentMessage);
}
await Send.SendSentEmote(context);
}
}
Expand Down
17 changes: 14 additions & 3 deletions Voltaire/Controllers/Messages/SendToGuild.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,27 @@ public static async Task LookupAndSendAsync(SocketGuild guild, ShardedCommandCon
return;
}

if(!IncrementAndCheckMessageLimit.Perform(dbGuild, db))
if (!IncrementAndCheckMessageLimit.Perform(dbGuild, db))
{
await Send.SendErrorWithDeleteReaction(context, "This server has reached its limit of 50 messages for the month. To lift this limit, ask an admin or moderator to upgrade your server to Voltaire Pro. (This can be done via the `!volt pro` command.)");
return;
}

var prefix = PrefixHelper.ComputePrefix(context, dbGuild);
var channel = candidateChannels.OrderBy(x => x.Name.Length).First();
var messageFunction = Send.SendMessageToChannel(channel, replyable, context, dbGuild.UseEmbed);
await messageFunction(prefix, message);
if (context.Message.Attachments.Any())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to do this check and the iteration inside of Send.SendMessageToChannel so we don't have this conditional logic everywhere? If we do go this route, I think the first thing to do here would be to determine if the supplied context has attachments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that this would be preferable, and it was in fact how I did it initially. It worked fine if a message had a single attachment, but when there were multiple, I noticed that the whole Func<string, string, Task<IUserMessage>> function is supposed to return only a single IUserMessage Task, and I couldn't really figure out a way around that (as the messageFunction is awaited all the way back in SendToGuild).
Doing the logic outside of the Async area was the easiest way I could find to make sense of it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - could we, for example, return a function that returns that last IUserMessage? Or an array of them?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as they are awaited inside of SendToGuild, we can't really just return the last one. It would have to be a list.
I'm not very familiar with Async flows so I'm not sure if it can be an ordinary list or if it has to be some kind of Async List.
My issue with using an Async List is that the attachments might have their order jumbled, which would be a problem if the message text was only attached to the first file.
If we do go that route, Task.WhenAll() should be enough.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - the caller is what is doing the awaiting, but I think all of the callers are essentially just awaiting the returned message. We may have to alter the inner workings, but we should be able to have this thing await the returned function and have it receive the result of sending the last message, I think.

I'm happy to take a whack at this at this week some point 🙂.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd appreciate that, I'm struggling a little to get my head around async, hence the current state of my changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, one more issue with just returning the last message;
sendReply and SendDirectMessage both add the Delete React to each message that gets sent. if you look at those, the Send.AddReactionToMessage is inside the loop, to let users delete each image individually.

{
for (int attachmentIndex = 0; attachmentIndex < context.Message.Attachments.Count; attachmentIndex++)
{
var messageFunction = Send.SendMessageToChannel(channel, replyable, context, dbGuild.UseEmbed, attachmentIndex);
await messageFunction(prefix, message);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a message is sent with the attchments, does the message end up being sent multiple times?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the case until I added the ternary operations in the SendFileAsync lines.
My implementation of the attachmentIndex was a lazy fix to retrieve the index of the current attachment that was being iterated, to ensure that the attachments were being send in order.
Initially, if the message was supplied with each request, it would repost the text alongside each attachment, however I used the attachment index attachmentIndex == 0 ? message : "" to remove it.

}
}
else
{
var messageFunction = Send.SendMessageToChannel(channel, replyable, context, dbGuild.UseEmbed);
await messageFunction(prefix, message);
}
await Send.SendSentEmote(context);
return;
}
Expand Down