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

Upgrade Discord.js #350

Merged

Conversation

steelskillet
Copy link
Contributor

updated discord.js to v14.14.1 (latest) and fixed any issues present with the plugins.

you will note few changes with plugins. this is because the major change that had to be done was making the embed part of a list which is handled in the base-plugin.

server-status had changes that needed to be made because it relied on the embedBuilder/Embed object, which has changed drastically, it now utilizes the json format which has had 0 changes from v12->v14.

Was quickly tested on Unnamed servers and all was working well.

@steelskillet
Copy link
Contributor Author

requires version change still

@steelskillet
Copy link
Contributor Author

steelskillet commented Mar 17, 2024

fix for discord-server-status.js, the activity type field wasn't changed to the proper input. set to custom activity so that it's easy to see the entire status text from the sidebar.
forgot to rename the commit message

@MartinezAye
Copy link

Have you got plugins to log to threads within channels?

@steelskillet
Copy link
Contributor Author

steelskillet commented Mar 17, 2024

yup, just tested it. threads are pretty much treated like channels from an API perspective from discord. so you just throw the thread ID in for where the channel ID is.
they won't make the threads on their own, you have to make them, but they do log to threads.

@werewolfboy13 werewolfboy13 added the Testing Required This requires testing before approval. label Mar 17, 2024
@werewolfboy13
Copy link
Collaborator

Adding flags, will need some QA on this so I can make sure its stable enough to make ready for public use.

@steelskillet
Copy link
Contributor Author

yup, currently running on all unnamed servers flawlessly so far.

@steelskillet
Copy link
Contributor Author

steelskillet commented Mar 22, 2024

potential changes to make so that plugins have a better chance of not needing changes.

  • parse the embed colors, discord.js seems to have changed what it likes for this.
  • mirror the messageCreate event to an event named message.

@steelskillet
Copy link
Contributor Author

side note that this PR fixes a known bug that causes squadjs/discord.js to crash when sending text in voice channels.

@magicoflolis
Copy link

@steelskillet So before in v12 you could await the login function which then resolves the whole Client class.

In v14 it is slightly different, after await connector.login() is resolved, the Client will not be completely ready, only once Events.ClientReady is emitted then the Client class is ready.

image

With current method:
image

In order to fix this we would need to wait until the Client is "ready" before returning the connector.

/squad-server/factory.js

await new Promise((resolve, reject) => {
	connector.once(Events.ClientReady, (readyClient) => {
	    Logger.verbose('SquadServerFactory', 1, `Ready! Logged in as ${readyClient.user.tag}`);
	    resolve();
	});
	connector.once(Events.Error, reject);
	connector.login(connectorConfig);
}).catch((ex) => {
	throw ex
});

Result:
image

The downside is the startup process takes slightly longer due to waiting for the client to fully be ready.

image

In addition to v14 , channels are cached in a Map so there is no need to await and fetch them.

/squad-server/plugins/discord-base-plugin.js

/** @type { import('discord.js').Client<true> } */
this.discordClient = this.options.discordClient;
this.channel = this.discordClient.channels.cache.get(this.options.channelID);

@werewolfboy13 werewolfboy13 self-assigned this May 10, 2024
@werewolfboy13 werewolfboy13 added core bug Bug related to the core SquadJS API major Major Change labels Jul 16, 2024
@IgnisAlienus
Copy link
Contributor

IgnisAlienus commented Jul 30, 2024

I've got to look at the code, but I'd be curious how difficult it would be to allow multiple channelIDs for a single DiscordBase plugin. I've wanted to do it on some of my custom plugins and would be looking to do it in the MySquadStats plugin.

Maybe one of y'all that have help with the PR may know off the top of your head the feasibility.

If somewhat easily possible, my recommendation would be to add support for it to this PR.

@steelskillet
Copy link
Contributor Author

What were thinking functionality wise with multiple channel ids, like if a bot was in multiple guilds for it to paste the same message to those different guilds?

@watchful-eyes
Copy link

I've got to look at the code, but I'd be curious how difficult it would be to allow multiple channelIDs for a single DiscordBase plugin. I've wanted to do it on some of my custom plugins and would be looking to do it in the MySquadStats plugin.

Maybe one of y'all that have help with the PR may know off the top of your head the feasibility.

If somewhat easily possible, my recommendation would be to add support for it to this PR.

discord-base-plugin.js
Visit SquadJS Discord to see the changes
maybe something like this'd suffice for what you want but I also didn't understand why you want such thing...

@IgnisAlienus
Copy link
Contributor

discord-base-plugin.js Visit SquadJS Discord to see the changes maybe something like this'd suffice for what you want but I also didn't understand why you want such thing...

Looks like it would work.

Right now you can only put one channelID. So if you're setting up a plugin, you're forced to have all discord messages sent to just that one channel.

I'd like to give plugin devs the option to give SquadJS users the option to split up messages that are sent. A quick, and pretty dirty example would be, maybe in the kill feed plugin, have all normal kill feeds sent to a public channel id, but we add a detection counter for certain kills over a delta to detect cheaters and have that sent to an admin only channel.

@IgnisAlienus
Copy link
Contributor

What were thinking functionality wise with multiple channel ids, like if a bot was in multiple guilds for it to paste the same message to those different guilds?

That could be a possible use case, but more of thinking single guild, different channels for different messages from one plugin. See above message to Watchful for a quick example.

TLDR, a plugin that would sent both public and admin messages but to different channels instead of having to build to plugins or duplicating a plugin in config.json that only has different values for channel.

@steelskillet
Copy link
Contributor Author

steelskillet commented Jul 31, 2024

I can see use in the basediscord plugin being able to handle multiple message channels. it might become pretty complicated though because we would need a way to keep track of an arbitrary amount of channels to send to, all using arbitrarily named versions of this.options.channelID. the main thing is figuring out which options are channels to load/cache and how to then reference them from plugins built off of basediscord. we could handle this.options.channelID as a dictionary (code reference name: channel id) but then the users would be able to easily mess up the names used in the config for referencing which channel we are trying to send to. this would still probably be the cleanest way to do it though. this.sendDiscordMessage() would then take message and channelName with channelName being optional, we can default to the first channelID. we could then parse the this.options.channelID if it comes as a string/channelid then we set that to default and only use that (ignoring the channelName parameter), if it's a dictionary we do the fancy stuff, this would retain backwards compatibility. and of course refactoring this.channel as a dictionary or adding a new this.channeldict and putting the default in this.channel. plugin devs would then assign the default value for this.options.channelID as a dictionary with <channelName>: <dummy value>.

So an example options for this might look like:

{
  "plugin": "DiscordChat",
  "enabled": false,
  "discordClient": "discord",
  "channelID": {
    "AllChat": 29738497283974298,
    "AdminChat": 647826478264782,
    "Team1Chat": 2938479237483,
    "Team2Chat": 2398482937489237
  },
  "chatColors": {},
  "color": 16761867,
  "ignoreChats": []
},

with the in-code example being:

await this.sendDiscordMessage(adminchattext, "AdminChat"); //send to "AdminChat" channel
await this.sendDiscordMessage(someothercontent); //send to "AllChat" channel

@werewolfboy13 werewolfboy13 added plugin bug Bug related to the SquadJS plugins minor Minor Change and removed major Major Change labels Aug 6, 2024
@werewolfboy13
Copy link
Collaborator

Internal QA validation requirements met. Holding off to push version bump until more data is given from general pop.

@werewolfboy13 werewolfboy13 merged commit 5e06b9a into Team-Silver-Sphere:master Aug 6, 2024
@Thomas-Smyth Thomas-Smyth changed the title The quick and easy Discord.js upgrade Upgrade Discord.js Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core bug Bug related to the core SquadJS API minor Minor Change plugin bug Bug related to the SquadJS plugins Testing Required This requires testing before approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants