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.

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

Roadmap: unified Channel class #7321

Open
kyranet opened this issue Jan 22, 2022 · 1 comment
Open

Roadmap: unified Channel class #7321

kyranet opened this issue Jan 22, 2022 · 1 comment

Comments

@kyranet
Copy link
Member

kyranet commented Jan 22, 2022

ℹ️ This goes hand-in-hand with #6567, and as such, the linked PR must be solved before this one.

❓ The problem

Currently, the way we handle channels in the library is overwhelmingly complex, for example, just to create the channel class, we need a lot of checks:

static create(client, data, guild, { allowUnknownGuild, fromInteraction } = {}) {
CategoryChannel ??= require('./CategoryChannel');
DMChannel ??= require('./DMChannel');
NewsChannel ??= require('./NewsChannel');
StageChannel ??= require('./StageChannel');
StoreChannel ??= require('./StoreChannel');
TextChannel ??= require('./TextChannel');
ThreadChannel ??= require('./ThreadChannel');
VoiceChannel ??= require('./VoiceChannel');
let channel;
if (!data.guild_id && !guild) {
if ((data.recipients && data.type !== ChannelType.GroupDM) || data.type === ChannelType.DM) {
channel = new DMChannel(client, data);
} else if (data.type === ChannelType.GroupDM) {
const PartialGroupDMChannel = require('./PartialGroupDMChannel');
channel = new PartialGroupDMChannel(client, data);
}
} else {
guild ??= client.guilds.cache.get(data.guild_id);
if (guild || allowUnknownGuild) {
switch (data.type) {
case ChannelType.GuildText: {
channel = new TextChannel(guild, data, client);
break;
}
case ChannelType.GuildVoice: {
channel = new VoiceChannel(guild, data, client);
break;
}
case ChannelType.GuildCategory: {
channel = new CategoryChannel(guild, data, client);
break;
}
case ChannelType.GuildNews: {
channel = new NewsChannel(guild, data, client);
break;
}
case ChannelType.GuildStore: {
channel = new StoreChannel(guild, data, client);
break;
}
case ChannelType.GuildStageVoice: {
channel = new StageChannel(guild, data, client);
break;
}
case ChannelType.GuildNewsThread:
case ChannelType.GuildPublicThread:
case ChannelType.GuildPrivateThread: {
channel = new ThreadChannel(guild, data, client, fromInteraction);
if (!allowUnknownGuild) channel.parent?.threads.cache.set(channel.id, channel);
break;
}
}
if (channel && !allowUnknownGuild) guild.channels?.cache.set(channel.id, channel);
}
}
return channel;
}

(And let's not mention it's also a class that requires quite a workaround to work, as seen below):

if (data && immediatePatch) this._patch(data);

Furthermore, changing channel types is not easy:

if (channel.type !== data.type) {
const newChannel = Channel.create(this.client, data, channel.guild);
for (const [id, message] of channel.messages.cache) newChannel.messages.cache.set(id, message);
channel = newChannel;
this.client.channels.cache.set(channel.id, channel);
}

On a large scale, those checks cause the shard ready quite slower.

✅ The solution

We can make a single class following whichever design we take with the aforementioned PR, change the extended classes to interfaces, and use #6957 to cast Channel to those interfaces.

Some stuff such as the line below:

/**
* The guild the channel is in
* @type {Guild}
*/
this.guild = guild;

Would just be a getter, following the design from the Message class:

/**
* The id of the guild the message was sent in, if any
* @type {?Snowflake}
*/
this.guildId = data.guild_id ?? this.channel?.guild?.id ?? null;

get guild() {
return this.client.guilds.resolve(this.guildId) ?? this.channel?.guild ?? null;
}

⬆️ Upsides

  • Lowers performance impact when instanting large amounts of channels in a short period of time (e.g. a bot on +10 internal shards).
  • Much easier type-changing mechanism - as easy as simply patching it.
  • Future-proofing messages and webhooks, PRs like feat(VoiceChannel): add support for text in voice #6921 would be significantly easier, or even not need to exist.
  • No mixins, makes documentation easier.
  • No need for inheritance trees, middle-ground shared classes (BaseGuildTextChannel, BaseGuildVoiceChannel, and GuildChannel) and future ones that might arise in the future.
  • (?) Less memory usage due to channel managers storing only 1 class as opposed to using a lot of different classes with different shapes (https://mathiasbynens.be/notes/shapes-ics).

⬇️ Downsides

  • A single class for everything means a very complex class.
  • How do we manage message caches? Should we remove them? Issue soon!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

2 participants