Implement !tunnel command

[Matiiss]

Fixes #3512

[Matiiss]

I do have a couple of questions though, first, so currently I have made it so that only discord.TextChannel are supported, which means you can't use this command inside those voice channel text channels or threads, but I suppose it'd be fine to allow its usage there, too.

Another thing is that if I change the command method signature to use discord.abc.MessageableChannel, then suddenly !tunnel is being interpreted as a "tag" and it's trying to find a match for it among the other tags, instead of running this command. I'd like some advice on how to make it behave like it does with discord.TextChannel or like make the tag search skip it or something.

Then also, if I use the discord.TextChannel annotation (and presumably it would be similar if I fixed it so that discord.abc.MessageableChannel would work), if I provide a wrong ID (same length, just changed some digit), it doesn't appear to complain at any level, it seems it just sets that argument to None, which, I'm not sure if that's the behavior that I'd like. That is, you either provide a correct argument for the channel or it uses the default behavior without letting you know that you may have messed up.

Should there be checks for whether the given channel is a DM channel or some such? Currently it just seems to treat it as incorrect channel ID (probably because it's not a discord.TextChannel), so it just sets it to None, but if switching to discord.abc.MessageableChannel, that should probably be explicitly checked?

Similarly, what to do about ctx.guild, should that throw some more specific error or should it realistically never be None (which is my assumption, hence the AssertionError)? I used raise AssertionError` mostly for typing purposes here (in places where it also should never be realistically raised anyway).

Is there a simple way to do overloads for this command? So that with one argument it's !tunnel [destination_channel], but then with two it's !tunnel [source_channel] [destination_channel]?

Any shortcuts/convenience functions I should use for say permission checks from the existing code base?

Does this need unit or integration tests?

[jb3]

Nice initial implementation, a few feedback comments. Will address your other comments in a second.

[jb3]

so currently I have made it so that only discord.TextChannel are supported, which means you can't use this command inside those voice channel text channels or threads, but I suppose it'd be fine to allow its usage there, too.

Sounds fine — with the necessary cooldown added as mentioned so users cannot spam this in places we have less visibility.

Another thing is that if I change the command method signature to use discord.abc.MessageableChannel, then suddenly !tunnel is being interpreted as a "tag" and it's trying to find a match for it among the other tags, instead of running this command. I'd like some advice on how to make it behave like it does with discord.TextChannel or like make the tag search skip it or something.

This sounds weird, do you get the same if you try use GuildChannel instead?

That is, you either provide a correct argument for the channel or it uses the default behavior without letting you know that you may have messed up.

I wonder if using a https://discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.ext.commands.TextChannelConverter might help?

Should there be checks for whether the given channel is a DM channel or some such? Currently it just seems to treat it as incorrect channel ID (probably because it's not a discord.TextChannel), so it just sets it to None, but if switching to discord.abc.MessageableChannel, that should probably be explicitly checked?

You can use a guild-only decorator to enforce this.

Similarly, what to do about ctx.guild, should that throw some more specific error or should it realistically never be None (which is my assumption, hence the AssertionError)? I used raise AssertionError` mostly for typing purposes here (in places where it also should never be realistically raised anyway).

Same as above.

Is there a simple way to do overloads for this command? So that with one argument it's !tunnel [destination_channel], but then with two it's !tunnel [source_channel] [destination_channel]?

Not easily without doing most of the argument handling yourself.

I was going to suggest that for this initial implementation though we ditch the source channel support. Especially since the user who did the tunneling isn't mentioned in the tunneling messages it seems ripe for abuse (e.g. someone spamming in bot commands with another source & dest).

For now, let's keep things simple and just implement an optional destination channel.

Does this need unit or integration tests?

You can if you want to (and if you remove the source stuff it should be easier), we do have some utilities for mocking Discord components here and it's not too hard to add new bits.

[jb3]

Also, on the last point about abuse: can we include the user who tunneled on both the source and destination message (username is fine, doesn't need to be a mention), so that we can easily see any problems without having to look through message logs.

[wookie184]

Thanks for working on this :)

Potential simplification:

1. Get the channels in the cog_load and store them in in a list e.g. self.channels
2. replace get_least_active_channel_id with get_least_active_channel that looks something like

   return min(self.channels, key=lambda c: thattimeutilthing(c.last_message_id)
   

3. Then the channel_id_to_timestamp logic + on_message aren't needed at all, and it's a bit simpler as you're passing the channel around instead of the channel id (don't need to fetch it).

[wookie184]

I don't think this will work. From what I can see the Discord API returns a last_message_id along with the channel, but discord.py requires the message to be in the cache for last_message to be populated. If the bot has just loaded, the message wont be in cache.

The easiest fix is probably to use channel.last_message_id directly with https://discordpy.readthedocs.io/en/latest/api.html#discord.utils.snowflake_time