SinisterRectus / Discordia

Discord API library written in Lua for the Luvit runtime environment
MIT License
688 stars 144 forks source link

Add support for text channels in voice chat. #355

Open TohruMKDM opened 1 year ago

TohruMKDM commented 1 year ago

This PR adds the GuildTextChannel class a base to GuildVoiceChannel which allows you to essentially treat a voice channel as a text channel which was made possible recently by discord introducing text chat in voice channels. It also makes the getChannel function found in EventHandler.lua search a guild's cache of voice channels which allows the proper receival of events for messages in these new channels. Such as MESSAGE_CREATE and MESSSAGE_UPDATE. Should no longer receive 2022-06-11 17:56:02 | [WARNING] | Uncached TextChannel (%d) on MESSAGE_CREATE warnings.

SinisterRectus commented 1 year ago

I know this is probably the easiest fix, and probably not likely to break things, but it is technically a breaking change. I wonder if we could just copy paste the relevant text channel code to the voice channel class.

TohruMKDM commented 1 year ago

The entire codebase for GuildTextChannel is now relevant to GuildVoiceChannels with the only exception being GuildTextChannel:getPinnedMessages() (at least based on my very limited testing) I was thinking you could just make it inherit the class and then just simply set that method to nil but that sort of seems, hacky? I'm not too sure about copying the relevant code unless duplicating code in multiple files is fine with you.

Bilal2453 commented 1 year ago

I'm not too sure about copying the relevant code unless duplicating code in multiple files is fine with you.

Not sure but: Member vs User

SinisterRectus commented 1 year ago

I'm okay with code duplication at this point. I'd rather do that than change the inheritance tree. 3.0 has one channel class and I expect it to stay that way so this will go away in the future.

Side-note: We can make sure this counts for Hacktoberfest if you are interested. I think that it will as long as I merge it during October.

SinisterRectus commented 1 year ago

As a reminder, we do not start tracking pull/merge requests until October 1st. PR/MRs created before then won't be tracked.

You would need to open a new one.

TohruMKDM commented 1 year ago

I finally have some time to get around to this but before I proceed, I would like to clarify on which approach I should be taking. It seems there is no way to do this that won't come off as a hack, should I be copying all of the methods from TextChannel and GuildTextChannel to GuildVoiceChannel? Or should I continue to make GuildVoiceChannel inherit GuildTextChannel and set the method getPinnedMessages to nil? There is also a third option of avoiding inheritance entirely and using some hacks from the debug library but I'm not sure if I really need to use debug for this.

Bilal2453 commented 1 year ago

No need for maintaining two copies of the same thing, in fact, it is fairly pointless when there is a more obvious way to do basically the same thing:

local GuildTextChannel = require('containers/GuildTextChannel')

local GuildVoiceChannel, get = require('class')('GuildVoiceChannel', GuildChannel)

function GuildVoiceChannel:__init(data, parent)
    GuildChannel.__init(self, data, parent)
    for k, v in pairs(GuildTextChannel.__class) do
        self.__class[k] = v
    end
    for k, v in pairs(GuildTextChannel.__getters) do
        self.__getters[k] = v
    end
end

(note, GuildTextChannel and GuildVoiceChannel would share the same pool, which should be fine)

This is technically all you need in the GuildVoiceChannel class. Now this won't handle MESSAGE_CREATE events, though nor would copying things from GuildTextChannel over to here. To handle this you will need to also change the event handler a bit nonetheless.

I also advice against undefining getPinnedMessages, leave it there so documenting this becomes easier, it will return a 400 if it was used on a voice channel, so be it.

Bilal2453 commented 1 year ago

P.S. it is probably a good idea to close this PR and open another one, for both the hactoberfest, and for the fact you will have to scrape all of those changes.

Bilal2453 commented 1 year ago

I know this is probably the easiest fix, and probably not likely to break things, but it is technically a breaking change. I wonder if we could just copy paste the relevant text channel code to the voice channel class.

Also, I am making the assumption here, that by "technically breaking change" you mean "this is technically a breaking change because it changes the inheritance tree"; and that the only difference between directly copying the code (or manually copying class and getters) and between inheriting it is changing the inheritance tree. Therefor this should be the proper way of "copying" the code.