#archive-library-discussion

25085 messages · Page 4 of 26

warped crater

okay, well, scrap what I said then, its not currently an issue, but keep it in mind for v13 👍

hollow garnet

I just want to point out an issue with User.displayAvatarURL(). If someone has an animated avatar, it still gets a .webp avatar instead of a .gif. Small thing, but I noticed it making an avatar command

unique axle

not an issue, expected behaviour if you do not provide dynamic: true

this was introduced in a semver minor update, which could not include breaking changes, meaning default had to be the same as before

hollow garnet

Oh, ok facepalm

unique axle

though that being said we could (and probably should) revise that for v13 and let it default to true

hollow garnet

I saw that the function was a thing and just used that, I didn't even bother checking the arguments

slate nacelle
unique axle

waiit, so should it have been defaulted all this time? potatodetective

slate nacelle

The idea of this option is to be able to specify the non-gif extension, as before it was only webp.

That is poorly worded, I think. Not that I can think of a better way.

unique axle

yes, but on the other hand if it used to default to display gif if it was animated then shouldn't that behaviour have been there as well after?

slate nacelle

Yes

unique axle

ablobcatsipsweats so dynamic = true

slate nacelle

No, that would be breaking too.

If you specify webp and not dynamic (or false) it should always be webp (regardles of whether it is animated).

Nothing provided -> webp or gif if animated
extension provided -> always extension
extension and dynamic (true) provided -> extension or gif if animated

If I understood that correctly.

It looks like we lost the first case after introducing it.

unique axle

aye, that is what i was trying to get to

<Guild>.iconURL() should be gif, if animated, because that's what it was like before and what makes sense (but when introducing this this was changed)

so that commit was a breaking change in minor and a fix for this would be minor? or how does that work

slate nacelle

So it is up to us, as usual theCutest

unique axle

v13 it is i guess, seeing that that should happen soon™️
(esp. keeping in mind that discord will rather likely start introducing the new permissions at some point (as was hinted at with stickers, for example))

slate nacelle

Wouldn't that be a rather trivial (although breaking) change?

unique axle

the v13 thing? ye
bigint bitfields.. bit scared to do that myself, i have no idea how bigints work shibaThinking

slate nacelle

Just BigInt(...), no?

vernal atlas
barren loom

UnhandledPromiseRejectionWarning: TypeError [INVALID_TYPE]: Supplied parameter is not a User nor a Role.
i think there should be or instead of nor

old kelp
barren loom

oh okay sorry. I am from germany 😄

lofty birch

Was the role#tags thing never merged?

vivid field

there's not even a discord-api-docs pr for that yet, so it might take a while until it's ready to get merged

lofty birch

Oh, I was under the assumption it was documented, my bad

unique axle
oak quail

what do we even want to do about that

would it be ok to release even if it doesn't work in all guilds?

indigo oar

so now users can have both EARLY_VERIFIED_DEVELOPER and VERIFIED_DEVELOPER in <User>.flags.toArray() ?

same for the partnered server owner one

just tested it

don't know if anyone has noticed that already

lofty birch

verified developer is deprecated

you should just switch to relying on the new one

indigo oar

I know but should it be removed?

lofty birch

it will in the next major release, i presume

indigo oar

aswell as DISCORD_PARTNER ?

well, okay, i guess

lofty birch

(ie v13)

copper laurel

Yeah we can't remove things in minor releases

brisk inlet

v12.4 seems to bugged when it comes to partial events.
"messageReactionAdd" doesn't work at all for users who are not previously cached.

I've downgraded back to v12.3.1 and it's all working correctly.

Edit:
Looks like someone found the problem. (you need the partial GUILD_MEMBER now):
https://github.com/discordjs/discord.js/issues/4921
If you plan to keep this you might want to note that on the change logs.

unique axle

If you believe you found a bug please provide a minimal reproducible code sample (plug&play preferable) so we can verify and investigate the issue over on https://github.com/discordjs/discord.js

real jetty

I dont understand why djs has added cache propriety to all Manager

left moat

Well its to get the cache of a manager

real jetty

It's to get values yes but i think it can be ommited

We can let dev ommit it

left moat

It makes sense from a design perspective, .cache to get from the cache

real jetty

But i think it can be cool to made Manager by default return cache

Exemple: client.guilds return collection of guilds by default

left moat

Well if you reallyreally want to do that you can fork discord.js or register your own manager

But thats a stupid decision that only lazy people will take because it's too much effort to add .cache

real jetty

Now i have explains ly opinion but what is yours ?

(Not only doge)

But thats a stupid decision that only lazy people will take because it's too much effort to add .cache
@left moat in a big bot it can be boring i think

And it permit to peoples who migrated from 11.x to made it easly

left moat

c'mon man

It takes like 2 hours at most

slate nacelle
lofty birch

In 12.4 channel parent IDs appear to always be null?

On 12.4 I can't find a single channel that has a parent ID

12.2 has them all correct though

Ah, how do I install that, discordjs/discord.js?

merry python

ye

lofty birch

Thanks

oak quail

is there a reason why User#locale exists? its for oauth2 and seems to be always undefined in djs

ornate topaz
oak quail

the property still existing in the library is an oversight
so maybe remove in v13?

copper laurel

Is it undefined for ClientUser too?

unique axle

yes used to send us for bots, doesn't do that anymore

oak quail

hm

vernal atlas

shouldn't it be null

oak quail

well it should be en-US

since thats what discord sends

unique axle

started sending it again?

oak quail

no point supporting it but ¯_(ツ)_/¯

vernal atlas

is that on 12.4 ? @oak quail

unique axle

waiiit at me is authenticated

oak quail

@vernal atlas some commit between 12.3.1 and 12.4

vernal atlas

ah

on 12.4 its set to null but on <12.4 its undefined FuzzyEyes

isn't that technically a breaking change FuzzyEyes

oak quail

even if we want to support it for ClientUser (which imo we shouldnt) it shouldnt be on User

i would be very surprised if that actually manages to break anything lol

vernal atlas

yeah i would be too but you never know what type of code someone has GuessIwillShrug

tacit crypt

@oak quail there's a difference between REST get user and the user we get on ready

warped crater

which is? kannaSus

that is news to me

tacit crypt

which is missing props

🤣

warped crater

what the fuck? what props are missing kannaSus

unique axle

locale, for example

warped crater

ah, so like, the stuff you'd get via oauth identify for a user, that you wouldn't be able to get normally, right?

ocean wedge
vernal atlas

i havent seen any mention of it or any PRs (assuming its a problem with the CI script anyway) but the CI seems to be broken aEyesShake

oak quail

@ocean wedge it does

well it should

if you can reproduce it not working, make an issue

unique axle
tight crag

Hey
i find a "bug" (likely)
On version 12.4.x, the shard system is a little broken, and shards themselves do not start if they are disconnected.
On version 12.3.x (specially rolled back) everything works fine. Is this a bug?
Now I don't even know whether to roll back to the old version, or what to do. My bot is quite large.

ornate topaz

any more context? does that happen for any reason or randomly? once per day or after 5 minutes from start?

tight crag

@ornate topaz I deliberately disabled one of the shards in order to test it (so that the all shards are not disabled, but only 1 shard). So I found such a "bug".

ornate topaz

soo what exactly did you do?

tight crag

I also tried to break one of the shards so that it disconnects itself due to an error. The same thing, it did not restart (although respawn: true is set in the ShardingManager parameters).

I manually, through eval, registered disabling the shard (process.exit()). On older versions, it restarted after that, now - no.

lofty birch

did you enable the toggle?

because apparently not doing that can cause the connection to just hang

the guild members intent toggle

oak quail

the toggle in the dev portal

lofty birch

bot

yeah

oak quail

intents determine what data discord sends you

partials is a djs thing, not a discord thing

lofty birch

partials was done by djs, intents by discord (entirely different things)

oak quail
slate nacelle

I just tested that config and the debug logs log very clearly that it was closed by Discord with 4014 (Disallowed intent(s)).
So I have no clue why it wasn't logging such.
The process also cleanly (or not, depending on whether the rejection is handled) exits then.

But glad that it's fixed anyway.

oak quail

no it isnt

partials and intents do completely different things

partials means djs wont drop events if data isnt cached

discord will only send events for the intents you have enabled

oak quail

voice states

unique axle

please use support channels if you don't discuss discord.js library development

slate nacelle
copper laurel

@oak quail Are you proposing a top-level option for #reply() to mention the user rather than making people directly specify the allowed_mentions option?

Because message#reply() otherwise follows the standard .send() mechanics of .send(content, options)

So your replied_user PR should already cover it

oak quail

my pr was to types

yeah i was proposing an option for #reply()

copper laurel

oh

Well I guess it depends, do we just make them specify .reply(content, { allowedMentions: { replyUser: true } })

Cause anything else is essentially a helper option

reply(content, options, boolean)

.reply(content, { replyUser: true }) for ReplyMessageOptions

which is otherwise a direct extension of MessageOptions lol

unique axle

hmm.. that too is kinda not nice catHMM

oak quail

.reply(content, { replyUser: true }) for ReplyMessageOptions
well replyUser doesnt make sense here

unique axle

that's mixing the options with options it propagates one tier of nesting further down which... seems a bit yucky

oak quail

putting it in allowed mentions is pretty weird tbh

unique axle

i mean it is a mention which also needs to be parsed and determined if it should mention, so it does make kinda sense
but fiddling with it and moving it into places where it doesn't belong seems... strange
though i do have to agree that this should probably be documented with at least an info bit and example

oak quail

also

how would djs behave when theres a mention in the mentions array which isnt in the msg content

i assume it doesnt check the content right?

unique axle

Message#mentions takes it from the payload

save channels, those are parsed from content

oak quail

channel_id is now optional on a message reference, meaning the only mandatory field is message_id
You can still only reply to a message within the same guild and channel
If you send channel_id, we will validate it (@meishuu why would anyone do this 🤔 )

  1. yay
  2. f
  3. Thonk

@copper laurel ^

copper laurel

Yeah I uhhh

That refers to a sent one?

So if not provided it assume same channel

oak quail

yeah

what do you guys think about making guild.owner a promise in v13 and making it fetch the owner instead of get from cache

copper laurel

guild.fetchOwner

real jetty

is this a bug or something? thonk

oak quail

does the bot have admin?

real jetty

@oak quail yes. Looks like it shows true for admins hmm

oak quail

yeah it probably doesn't check if the permission is valid if the member has admin

make a bug report

real jetty

sure

ornate topaz

hm, from what i played around, simply changing order in the return in Permissions.has solves 3 / 5 incorrect true results. Seems like it would need to actually check the flags to solve all cases

snow crypt

@vernal atlas in the v8 update, why not have all bit fields use bigints? why have it inconsistent?

snow crypt

also, the switch to npm ci broke all workflows

ornate topaz

all? no

snow crypt

fine

all workflows that use it

Run npm ci
  npm ci
  shell: /bin/bash -e {0}
npm ERR! Error while executing:
npm ERR! /usr/bin/git ls-remote -h -t ssh://git@github.com/discordjs/docgen.git
npm ERR! 
npm ERR! Warning: Permanently added the RSA host key for IP address '140.82.112.3' to the list of known hosts.
npm ERR! git@github.com: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.
npm ERR! 
npm ERR! exited with error code: 128

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2020-10-29T09_37_26_682Z-debug.log
Error: Process completed with exit code 1.
cold basin

@solemn oyster yeah those changes are cool but I think that would be something for an actual new version of djs, like v13 or even v14. removing 'convenience' like that would break a LOT.

solemn oyster

That's what this label is for, sir

cold basin

still

solemn oyster

Anything marked as semver: major is to be released in the next major release following semantic versioning's rules, that is, v13 or v14 in our case

cold basin

oh

I see

solemn oyster
cold basin

yeah in that case I'd fully support this

solemn oyster

We can't really break anybody's bots in v12, in fact, my bot updated just fine from v12.3.1 to v12.4.1, not a single error, and that's a huge bot written in maxed strict TypeScript with 57 thousand lines of code.

Not to flex, but it uses almost the entire discord.js interface, and a small change can result on a compiler error, but surprisingly, it didn't this time.

That's because we're using semver (Semantic Versioning), we can't release breaking changes under 12.x.x

cold basin

alrighty

ornate topaz
snow crypt

well then i really have no idea

ornate topaz

seems to be more related to the docgen repo than npm ci

since the one i linked doens't touch that

solemn oyster

If you ask me, I always found npm ci completely unnecessary

actions cache + yarn completes in milliseconds

ornate topaz

wasn't npm ci made to just flat out "restore" from package-lock.json

solemn oyster

Yes

ornate topaz

instead of building trees etc

solemn oyster

But it's still slower than caching node_modules and skipping the entire install step if the lock file is unchanged

vernal atlas

@snow crypt i considered that, but then again whats the point in converting it between numbers and bigints if it doesnt need to be converted

lofty birch

Hey
i find a "bug" (likely)
On version 12.4.x, the shard system is a little broken, and shards themselves do not start if they are disconnected.
On version 12.3.x (specially rolled back) everything works fine. Is this a bug?
Now I don't even know whether to roll back to the old version, or what to do. My bot is quite large.
This (<#archive-library-discussion message>) just occured to me also. One of my shards died with no errors or logs about it, and would not restart automatically.

the shard had only been on for ~1h and suddenly died

Even calling ShardClientUtil#respawnAll doesn't respawn that shard

Only thing that brings it back on is restarting the manager

vagrant holly

Automatic restarts seem to work fine for me. just ran process.exit on a shard and it cleanly restarted itself

lofty birch

Yeah, the shard had already died when I tried that

vagrant holly

Ah, I see the issue on gh

lofty birch

Oh, could you link me?

vagrant holly

oh lul what

Oh this is a regression in 12.4.0

dead

vernal atlas

where exactly are you pointing to

vagrant holly

Oh ew github deeplinks are shit

vernal atlas

the undefined being removed from this._exitListener = this._handleExit.bind(this, undefined); ?

vagrant holly

yah

vernal atlas

i tested that though and it didnt seem to have any effect it being there or not

vagrant holly

it definitely does have an effect

w/o it exit codes get passed to _handleExit

w/ it, undefined is passed so the correct respawn bool is used

vernal atlas

im confused

const fn = ((param = true) => param).bind(null, undefined);
console.log(fn()); // true, not undefined
vagrant holly

yes

vernal atlas

thats what was in the comment replying to vlad

vagrant holly

thats the behaviour we want

vernal atlas

yes but undefined that works the same

it still returns true

vagrant holly

undefined was removed

vernal atlas

and with undefined it still returns true

tacit crypt

except if you call it with a close code

vagrant holly

^

vernal atlas

im super confused

tacit crypt

fn(0)

vernal atlas

???

vagrant holly

fn2

my dude

vernal atlas

oh

tacit crypt 
class A {
  static onExit(respawn = true) {
    console.log('should respawn', !!respawn);
  }
}

on('exit', A.onExit) // will log false if exit's close code is 0
on('exit', A.onExit.bind(A, undefined)) // will log true since it's defaulted
vernal atlas

guess im just an idiot DogKek

tacit crypt

see I knew something would break when I asked about the change

vagrant holly

but yeah, unless a process exits via an error, a shard wont respawn in 12.4.x at present

vernal atlas

creating 2 bugs from the same PR FeelsBadMan

tacit crypt

2?

vernal atlas

yes

vagrant holly

Oh was 12.4.1 a regression from that PR too?

f

vernal atlas

yes

vagrant holly

what the shit is github actions doing

tacit crypt

Apparently not working...and it should be working...

vernal atlas

i think because it was changed to npm ci its borked

idk i saw someone say that was the issue, but im not sure

tacit crypt

no..the git cmd is erroring

even though it shouldn't..

vagrant holly

lemme npm i locally

vernal atlas

i think i actually made a branch to test around with it but got distracted

vagrant holly

ah yeah the lock file is cucked

tacit crypt

can you uncuck it

how is it even cucked

vagrant holly

yah 2 secs

vernal atlas

does my v8 pr just need (extensive) testing now? or is there more i need to add

oak quail

i'll see if i can test it sometime

looking at the or desc, it's pretty much just drop-in right? no major breaking changes?

vernal atlas

Permissions uses bigints and not numbers

so that would be a breaking change there

tacit crypt

well yeah but no

since its the internals, the user facing funcs still work the same

vernal atlas

Permissions#bitfield is public though

tacit crypt

well yeah THAT would be breaking

oak quail

yeah ik bitfield is a breaking change but like, I doubt many people are using it lol

tacit crypt

Directly? Most people fail .has kappapride (obv /s)

vagrant holly

Hm

So the current lock file is v2

vernal atlas

it would be the only way to check if a role had no permissions right?

vagrant holly

I guess I should retain that right

oak quail

ah someone was using node 15 I see

vernal atlas

thats what i use it for

oak quail

pre node 15 uses v1

tacit crypt

Just push updated file

dw about version

vagrant holly

Yeah, package.json says node 12+

oak quail

node 15 with lockfile v2 broke del's gh actions too lol

vagrant holly

probs cause actions node is 14 lts by default

vernal atlas

options.ws.intents is also removed in favour of options.intents & options.intents is required

so yea def a breaking change

oak quail

ah, missed that

vernal atlas

i should also add an error actually if intents arent passed

oak quail

what does it do rn

gateway connection error or something?

vernal atlas

yeah it just leaves it for the gateway to error

also setPresence and the helpers using it no longer return promises

because there wasnt a need for them to return one anymore

oak quail

isn't that kinda inconsistent tho

copper laurel

that confused me, are they not API calls?

oak quail

gateway commands

vernal atlas

theres not really anything it waits for

the only reason it was async is so the application could be fetched if you provided assets for the presence

which according to the docs you cant do anymore

only through rpc

oak quail

that was never possible 🤔

vernal atlas

well idk why it was even added then

but theres not really anything it actually waits for so idk what the point is in it returning a promise

vagrant holly

Just as an aside, 12.4.x had some nice memory usage improvements (I was running 12.3.1 w/ my message edit limit patched in). No idea what else changed w.r.t to mem usage, but something seems to have done.

copper laurel

12.4 specifically or discords intent changes?

vagrant holly

I was using intents prior, so it seems like 12.4 -- the drop in mem usage & the associated annotation is me rolling out 12.4.1 to the bot

tacit crypt

that's.. odd

but always welcomed

tender field

Hey, using djs 12.4.1, i'm not supposed to have a more complete error thanks to the new async queue? This is after a failed guild.fetchBan(id) (i'm not asking for help, this is a genuine question, because i thought the async queue would give more descriptive error when a promise fails)

DiscordAPIError: Unknown Ban
    at RequestHandler.execute (/bot/node_modules/discord.js/src/rest/RequestHandler.js:154:13)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at RequestHandler.push (/bot/node_modules/discord.js/src/rest/RequestHandler.js:39:14)```
tacit crypt

are you sure you're on v12.4.1? (odd question i know, check the code itself)

vernal atlas

looks alright for me

tender field

are you sure you're on v12.4.1? (odd question i know, check the code itself)
yep, checked on the node_modules folder...

vernal atlas

are you sure your bot is using 12.4.1?

perhaps try evaling / logging require('discord.js').version

tender field

well it is in the package.json and on the node_module folder

ok

yep it prints 12.4.1

it is transpiled with babel btw, idk if it changes anything

but i'm not sure if it comes from .fetchBan(id) or .unban(id), because the try/catch is the same for both and... you know i don't have the line ^^

well i can't reproduce this because it being called was a bug anyway, and i fixed it... but it is still weird that it did that
I managed to reproduce it, and yep, still a small stack trace

Same result with Node 14.15.0 and 12.18.3

vernal atlas

could it be because the request is being retried?

tender field

wdym?

wait was it for me or for the team? 😂

real jetty
tacit crypt

That's..the point

We have a bunch of those shortcuts (most are just mere permission checks)

real jetty

I proposed a getter to know if a channel is set as 'private' with the private icon but it was denied as it is an easy check... Will some getters diseapear in v13 ?

vernal atlas

yeah i believe that some of the getters will be removedf

frank turret
frank turret

@vivid field for your pr (#4935), why don’t you just return the cache itself instead of creating another collection?

vivid field

because the roles aren't on the cache in case cache is set to false

frank turret

Good point

vivid field

that's also what every other manager does meguFace

vagrant holly

Is a release gonna be cut soon given the regression fix was merged?

unique axle

as per usual: no ETA on releases

tender field

Will djs 13 provide some mocks or a testing api ?

real jetty

I would doubt it:
If you are PRing the lib, chances are you are only providing fixes or extending support to a specific endpoint, writing your own tests should be easy enough

tender field

no i meant as a djs user, not a djs contributor. To create tests for your bot

unique axle

no

storm igloo

Unoffical blabla, but it's very useful in recreating behaviour of a 3rd party api (like discord)

tender field

thanks

surreal dagger

hey, when v13 is coming?

unique axle

as per usual: no ETA on releases

real rivet

right, that's how it is
decent ways of implementing pagination will probably be a thing of the far future

copper laurel

Things which exist as complex concepts / multi-step processes (such as pagination) rather than wrapping singular API calls will likely never be a part of discord.js

real jetty

@oak quail yeah your right, its in there but not in the code

empty viper

Hi, I have question, when does the v11 will stop working ?

On the guide I says October 2020, but we are in November and my bot stills run on v11 (it's a test bot) 👀

oak quail

probably sometime in 2021

but people are already having some issues with it

ornate topaz

Did discord reschedule domain change?

cold basin

domain change did happen iirc

ornate topaz

Not exactly what i asked about

For now both domains work - that's what i asked about

unique axle
ornate topaz

So most likely in few days, not in 2021

unique axle

These are the statements, requiring the new domain later this year for third party developers on November the 7th 2020.
While there have been talks about re-directing and keeping discordapp.com alive, that is the public facing information.

In the end: we'll have to wait and see

You definitely should be updating to v12, not only because v11 might break due to the domain change but also because it does not receive new features or bug fixes anymore.
We have moved on from v11, it's an old version, we do not maintain or support it any longer. Please update at your earliest convenience catThumbsUp

empty viper

Okay I see now.
(PS : As I say it is just a test bot, it doesn't even have commands x))

drifting knot

first off is the diagnostics channels, could this change the debug event in the future? djs next?

there's also an event emitter when a child process spawns successfully, maybe relevant to sharding??

copper laurel

Not sure I see the benefit of the diagnostic channel over the existing debug event, unless it happens to be more performant. The spawn event definitely looks useful but again, I'd be hesitant to enforce a minimum Node version of 15.1

thorny tree

why can't the shard resume the connection, if the internet fails, and the connection closes with a 1000 code?
isn't this the point of resuming connections, if the internet fails?

If I normally connect:

[WS => Shard 0] [IDENTIFY] Shard 0/1
[WS => Shard 0] [READY] Session 49b63190f****************** (censored)
[WS => Shard 0] Shard received all its guilds. Marking as fully ready.

if I turn off the network:

[WS => Shard 0] [HeartbeatTimer] Sending a heartbeat.
[WS => Shard 0] [CLOSE]
    Event Code: 1006
    Clean     : false
    Reason    : No reason received
[WS => Shard 0] Clearing the heartbeat interval.
[WS => Shard 0] Session ID is present, attempting an immediate reconnect...
[WS => Shard 0] A connection object was found. Cleaning up before continuing.
    State: CLOSED
[WS => Shard 0] [DESTROY]
    Close Code    : 1000
    Reset         : false
    Emit DESTROYED: false
[WS => Shard 0] WS State: CLOSED

and activate it again after more than 30secs, it doesn't resume the connection, but makes a new identify call, which is very expensive

[WS => Shard 0] [CONNECT]
    Gateway    : wss://gateway.discord.gg/
    Version    : 6
    Encoding   : json
    Compression: none
[WS => Shard 0] Setting a HELLO timeout for 20s.
[WS => Shard 0] [CONNECTED] wss://gateway.discord.gg/?v=6&encoding=json in 1489ms
[WS => Shard 0] Clearing the HELLO timeout.
[WS => Shard 0] Setting a heartbeat interval for 41250ms.
[WS => Shard 0] [IDENTIFY] Shard 0/1
...

shards would've been destroyed within 30 seconds, afaik.
because of that, the shard gets destroyed with its sessionID and it does a new identify call (rather than resuming the connection)
is this a bug or a feature?

surreal geode

It looks like the socket closed with 1006 (without close frame), d.js was going to try to reconnect and resume but the socket was closed, so it reconnected and started a new session.

@thorny tree Why do you think starting a new session is expensive?

thorny tree

@surreal geode
because there is a 5s rate limit for identify calls and it needs to build the whole djs structure again

surreal geode

The 5s identify ratelimit applies to resume identify calls, too

The internal d.js structure is already "built" before you log in; the caches are filled but that isn't very expensive

thorny tree

The 5s identify ratelimit applies to resume identify calls, too
@surreal geode no I asked on the offical discord developer, and they said, it doesn't have a rate limit, and I can indeed confirm it after some tests

surreal geode

Huh. Sorry about that. I guess the 5s limit could be a problem but it seems you are only starting a single shard

thorny tree

30 shards reconnect in a few seconds rather than 2,5minutes
(or more extreme: 400 shards reconnect in a few seconds rather than 33minutes)

ornate topaz

(at 400 shards you would have higher max concurrency, but that doesn't change much here)

tacit crypt

Your session would expire anyways if you don't resume in time

There's like a very short window in which you CAN resume

old hill

Hi all. I was looking through the code recently, more specifically the rateLimiting things, and found that (iirc) the ‘rateLimit’ event is called when an event is delayed to avoid ratelimits, rather than when an actual ratelimit is hit - in which case a debug is sent. To me, this seems kind of counterintuitive, and I feel like at least the documentation should clarify this. Is there any particular reason it’s set up this way?

ornate topaz

when an event is delayed, you have hit a ratelimit

old hill

I can understand that, but I feel like the behaviour should be clarified in some way

thorny tree

There's like a very short window in which you CAN resume
@tacit crypt its more than 5 minutes, so it should be possible to resume all shards in a few seconds

tacit crypt

No it's not

Sessions that are DCd last max around 1 minute

Otherwise you'll get an invalid session

thorny tree

wait I'll test it

oh you are right, but a minute should be enough right? @tacit crypt

tacit crypt

Without a proper example/test, I don't see how it's a problem
We tried reconnecting with the session ID, failed, so now we're reidentifying since session is aasumed to be expired

oak quail

why does ClientPresence have all of this when bots can only set name, type, and url

drifting knot

i have the typescript code written and commented n everything

function findShardIDByGuildID(guildID: Snowflake, shardCount: bigint): number

where should i pr this method into

it could make sense to make it a method on Guild, however 99% of the use cases are for finding a shard for a guild where the only data you have is the id

so maybe it should be a static method on Guild or Util or Client???

warped crater

fellas we have got to have a discord js method that implements the formula ...

no..? we.. don't?

you never specified the need for it 🤔

if you want to find what shard a guild is on you can simply use an if condition inside of broadcastEval

this isn't really particularly useful besides avoiding the use of broadcast and just running the code straight on the target shard (if that's even possible atm)

and I see no need for it inside the library itself.

drifting knot

ok imagine this

imagine i'm mee6 and for whatever reason i'm using discord.js to run my 8660 shards

now imagine im caching data in redis that uses the guild and shard id

now every time i want to fetch that data execute 8660 eval statements to find the 1 shard that matters

this is literally a thing mee6 does, only not with discord.js

it makes sense for the discord library to implement the formula discord gives us to use the discord api

lofty birch

You could just create the function yourself?

drifting knot

i did

lofty birch

Given how it's so simple

drifting knot

and i am offering to PR it

it is easy to make mistakes in the implementation

you could argue that everyone could make their own discord api wrapper and discord.js doesnt need to exist but that is stupid

we are here to make the discord api more accesible to javascript developers

why not make this formula part of that

lofty birch

The purpose of the library is to interact with the api. Using the formula doesn't do that in any way

drifting knot

and yet it is listed on the discord api docs

discordjs has methods for escaping markdown, that isnt api related

is it even really your place to decide if this should or shouldnt be in the library

just tell me where to put the function, i'll pr it, and someone will approve or reject it

unique axle

ShardClientUtil or the general purpose Util shibaThinking
i personally feel like the first represents the area of use better
though i'm unsure i understand the use case. if i'm not mistaken (possible, i'm not very involved in sharding internals) you'll need to #broadcastEval it anyways and returning if a guild is on the respective shard or by applying a formula doesn't seem to make much of a difference

drifting knot

the use case is to remove the need to evaluate code on all your shards

it uses a formula thats the inverse of what discord uses for sharding (not big bot sharding)

unique axle

i understand that. the question is: do we even allow to address a single shard at this time

drifting knot

ehhhh not easily

but having the function in whatever util class would be good

its simple enough its like 20 lines to implement, 20 lines to test, and then forget it forever

ooh and 1 use real world use case i actually have for this is to see how many guilds each shard is allocated for me, where my bot uses each cpu core to handle a bunch of shard IDs

sage owl

So, I was working on my bot today and when going to test the messageReactionAdd and messageReactionRemove event portion of the code, i am not getting anything that would suggest this event firing. This event has been working for over a month and then all of a sudden stopped working.

I am using partials to grab the message that wasn't cached but the important fact is i am not seeing a console output that i specifically placed at the top of the function. Any ideas what could be causing this?

unique axle

bughunter Bug:
• MessageReactionAdd does not fire despite proper partials enabled

PR in the works,
Temporary fix is to ensure the addition of GUILD_MEMBER partial in the client definition
new Discord.Client({ partials: ['USER', 'GUILD_MEMBER', 'CHANNEL', 'MESSAGE', 'REACTION'] });

(copy from #769862166131245066)

sage owl

How long has this been going on, because this is the first i have seen this?

Is this bug due to discord changes or is this specifically a Discord.js bug?

Ok, that makes sense! Yeah, probably should too. Thanks for the info. I just found the discord after searching the net for this issue. IS this going be the best place for announcement and changes?

unique axle

well yes, this is our (discord.js) discord server

sage owl

Yeah, I just looking or the place where I will get the announcements first. Some API devs like their website to be the front page, others like GitLab or GitHub. I work in the industry, so was just curious where the best place to go is. I appreciate the help!

btw, I think you all are doing a stand up job with Discord.Js...keep up the hard work!!

unique axle

GitHub is where the issues and new features will be discussed, if we spot things happening regularly in support they'll go into #769862166131245066 , feature releases happen in #announcements
both you can follow if you prefer to get notified on your own server catThumbsUp

oak quail

btw re the sharding formula, I use it sometimes to see what shard a guild is on when the bot is offline in it

drifting knot

that too

vagrant holly

There isn't currently a way to broadcast to a single shard, but that seems like it'd be a super useful change

Will take a look shortly

wide sand

I have come across a very weird issue i think it;s djs issue.
the clientOption for fetchAllMember get timeout even when server member intent is enabled and bot is in 600 servers with 100K members

to test it properly i tried manually adding all intent double check server member intent

tried on different vps too

make sure to on warn and debug event to see it

once it timedout the client will go in ready state but no commands will work

vagrant holly

🤔 The websocket is closing

wide sand

yes which should not be happening

tacit crypt

So you're using intents and you have server member intents enabled in the dev portal?

wide sand

yes

tacit crypt

and whats your intent bitfield?

wide sand

i am using flags: 'GUILDS','GUILD_MEMBERS','GUILD_MESSAGES','GUILD_MESSAGE_REACTIONS'

    fetchAllMembers:true,
    ws: { intents: ['GUILDS','GUILD_MEMBERS','GUILD_MESSAGES','GUILD_MESSAGE_REACTIONS'] }    
});```
tacit crypt

d.js version?

wide sand

12.4.1

tacit crypt

ALSO, I'm pretty sure, on recent versions, it was moved outside of ws

vagrant holly

Intents doesn't take strings?

It takes a bitfield

tacit crypt

it does

oh

vagrant holly

🤔

tacit crypt

that too

also this is less lib discussion and more djs-help but I'll give you a pass

You forgot the constructor for the Intents class

vagrant holly 
        ws: {
            intents: (new Intents()).add(
                Intents.FLAGS.GUILDS,
                Intents.FLAGS.GUILD_MEMBERS,
                Intents.FLAGS.GUILD_MESSAGES,
                Intents.FLAGS.GUILD_MESSAGE_REACTIONS,
            ),
        },
tacit crypt 
new Intents([])```*
lofty birch

you can use an array of strings

I do

unique axle

what...

vlad, pls

  1. no, 12.4.1 is still under the ws key
tacit crypt

I thought we moved it what

oh

unique axle
vagrant holly

Nice, TIL it can take strings

@wide sand Long story short, ws: { intents: new Intents(['GUILDS','GUILD_MEMBERS','GUILD_MESSAGES','GUILD_MESSAGE_REACTIONS']) }

tacit crypt

No..thats fine

Doesn't NEED new Intents

since we resolve them

Well.. leaving aside that (@unique axle wasn't it a PR for moving intents up), it should work... Can you log client.options.ws.intents after you create the client instance (aka before logging in)?

wide sand

tried that too before

unique axle

Can you log client.options.ws.intents
that one - on the other hand currently doesn't work catLUL

vagrant holly

Oh wait, does intents itself get resolved too?

wide sand

same this:

Error [GUILD_MEMBERS_TIMEOUT]: Members didn't arrive in time.
    at /root/Node Apps/Menhera Chan/node_modules/discord.js/src/managers/GuildMemberManager.js:317:16
    at Timeout._onTimeout (/root/Node Apps/Menhera Chan/node_modules/discord.js/src/client/BaseClient.js:83:7)
    at listOnTimeout (internal/timers.js:554:17)
    at processTimers (internal/timers.js:497:7)
tacit crypt

yes, Matt

vagrant holly

So much resolving

wide sand

it works fine for bot that is small in less number of server. but don't for slightly bigger bot. this error i am getting is in 600 server bot

with 100K members

so it's djs issue?

vagrant holly

It seems like the library gives a guild 120s to return some members, or that error is thrown

Which feels like it should be given a setting

But also from your logs the WS was dying, so the timeout doesn't seem to be the issue

I assume if you disable fetchAllMembers the bot starts fine without the WS dying?

wide sand

yes

if fetchAllMember is false(default) bot starts just fine

but when it's set to true the WS closes

and get timeout

so there is no solution right now?

vagrant holly

I defer to someone who knows the internals of intents better than I

My only suggestion would be, as the docs note, to avoid using the fetchAllMembers client option

wide sand

but then i can't cache all members and members.fetch takes time

when the server is huge

lofty birch

you should avoid caching all members

real jetty

Why does Message#member returns null even if the message is sent in a guild ? Why doesn't it return a partial member if it can't find it ?

keen cosmos

it searches the cache for the member, and if it's not found it returns null

This is also technically a question, and belongs in a help channel, not here

real jetty

I'm talking about how the library behavior that seems weird imo

why doesn't it add automatically the member to the cache if it can't find it (at its creation) ?

With new intents if you disable guild members intents Message#member will be null if you don't fetch it, it's a problem imo

vagrant holly

Imo it'd make sense for it to return a partial

keen cosmos

Then you'd have to have a partial guild member in the cache

Message#member is a getter

which retrieves from the cache

real jetty

It is not a problem

I'll make a PR

ornate topaz

There already is one regarding that

real jetty

oops didn't see it

sry

vagrant holly

Am I going mad cause I don't see it, unless it was already merged

real jetty

But it is only for events

vagrant holly

Yeah, that doesn't actually ensure Message#member can be a partial

Just does a better job of populating the cache with partials

slate nacelle
vagrant holly

Right, but if the member/user had since been purged, Message#member will return null, not a partial, I think?

Message itself could store the raw ID, and could then repopulate the partial when Message#member is called

tacit crypt

Which feels like it should be given a setting
It is

It seems like the library gives a guild 120s to return some members, or that error is thrown
We refresh the timeout every time we get a GUILD_MEMBERS_CHUNK payload

As for WHY it errors... I don't know...Make triple sure your d.js is at the version you said, try different bot tokens, etc

vagrant holly
tacit crypt

doesn't mean we don't provide the option to change it and let's be honest... if payloads don't come in 120 seconds.. We have an issue

vagrant holly

Yeah, agreed it shouldn't affect anything, but there isn't a setting for it 😛

tacit crypt

ALSO

peculiar notice

..are..we..

double requesting all members

vagrant holly

double requesting all members
@tacit crypt Where?

ornate topaz

(that wasn't the pr i was talking about btw)

tacit crypt

I mean the error Chota reported comes from the WS manager

oh

my god

I think we are

vagrant holly
tacit crypt

No

Not that

look at the prefix

[WS => Manager]

I

..I have some testing to do but I think we're double fetching members

vagrant holly

Oh you're totally right we are

tacit crypt

wait no

I'm actually a bot

client.ws.status === Status.READY

n v m

vagrant holly

is status not ready during the initial fetching?

tacit crypt

nop

it's set to NEARLY

vagrant holly

Ah yes ic

tacit crypt

It..should..work tho

I do this in @signal lintel for now, I'm planning on changing that asap

vagrant holly

Are there any situations where you physically cannot request all members from a guild

tacit crypt

yeah, if you don't have the guild_members intent

or.. y'know, verified bot without it

wide sand

@tacit crypt i have guildmember intent enabled and my bot is 600 servers

i tried the same in small bot it worked in that

didn't timeout at all

but in big bot it timedout

tacit crypt

are you 100% sure you're on 12.4.1? Some caching issues can cause that

wide sand

yes

i updated today itself

tacit crypt

Oddd

wide sand

"discord.js": "^12.4.1",

i checked pakage.json just now

to make sure once again

oak quail

@tacit crypt its gonna be moved out of ws in v13

would be breaking to move it in 12

tacit crypt

well yes but no if you just deprecate the ws one

copper laurel

yeah you can add it to top level client options without removing ws

wide sand

DJs is using V6 right

copper laurel

v7 technically, same API spec as v6 but with better errors

wide sand

Aah

So it can be added outside ws in current djs version

Or wait till v13 comes out

copper laurel

that has nothing to do with the API itself

We can provide the ability to receive the options wherever we like

Just cant remove the existing placements until v13 because thats breaking changes

unique axle

unless - horror ensues - we accept both kek

  • that being said, still not sure i'm personally a fan of moving this specific one at all
lofty birch

I agree, I think it makes sense to keep it inline with the ws section

Given how that's what it affects - the websocket

oak quail

normally everything the library parses is up in ClientOptions and http and ws are only for advanced settings that are sent directly to discord without modification, which aren't meant for normal users

unique axle

the planned rewrite to typescript (no release version/roadmap/date estimate, own repository at discordjs/discord.js-next) is planned to be much less monolithic, swapping out cache provider/rest/gateway for own extensions/implementations will be much easier - however no reason for great excitement, it's in a pre-planning phase (very very very early)

catThumbsUp

vivid lodge

👍

I hope you plan on making your event methods easier to work with aswell, I had to go through so much shit to get custom events as a keyof interface in typescript

copper laurel

Extending ClientEvents is "so much shit"?

tacit crypt 
declare module 'discord.js' {
  export interface ClientEvents {
    myCoolEvent: [User, WoahAClassOfMineOwO, SoujiIsSuperCute, BlameVlad];
  }
}```

that is not so much shit, it's do it once and call it a day

vivid lodge

I had so many issues doing that tho

tacit crypt

such as?

wispy forum

Within typings of the library, as of 12.4.1, options are mandatory for methods like reply, send or edit. How come that is so? Back in 12.2.0 they were able to be undefined. Why exactly was this change made and how much difference would changing them back to be able to be undefined bring?

ornate topaz

Mandatory?

wispy forum

One sec.

Mandatory as unable to be undefined. My bad.

ornate topaz

So you say that it screams at you for not passing them to methods?

wispy forum

Which used to work in 12.2.0.

I'm just confused.

copper laurel

The first typing in that list does not have/require options, therefore there is an overload where they arent required

wispy forum

Alright. Still don't understand the desperate need to not make those optional for sake of edge cases but okay. I'm sorry for being irrational or annoying.

copper laurel

...but they are optional

wispy forum

Yes, I see the top one that solely has content as argument, I'm aware of it.

But I still think these bottom ones would be nice as optional.

copper laurel

You seem to not understand how method overloads work

Okay, credit to @ornate topaz for pointing this out to me, you (and I) were mistaking "optional" for undefined

The overloads do not explicitly support passing undefined in place of an optional parameter, because undefined isnt one of the accepted typings for that field if optionally provided

I messed up testing that with eval because JS doesnt care of course

Only TS does

wispy forum

Yes I kind of mess the two up sometimes during talking but I see now.

copper laurel

As did I, so apologies for being stubborn haha

wispy forum

All good, but guess that clears everything up.

solemn oyster

@wispy forum you need to somehow port all overloads those methods have, and then you can do options as any

While any is disgusting to use, it can be used to tell TypeScript that the data is fine

wispy forum

Yeah, there's a lot of times when handling types looks really ugly but has to do the trick ig

solemn oyster

So yeah, your issue isn't about it being potentially undefined

But rather about you passing all possible overloads to an overloaded method

wispy forum

I see that now.

solemn oyster

TS can't match all the overloads we have as a single one, so that's where this kind of issues happen

left moat
unique axle

the channel is for discussion, if you have nothing additional to add to the issue that requires discussion refrain from bumping, thanks. We really don't need this to become bump central.

vernal atlas

should these error codes be added into djs? i think they're all for store-related things that djs doesn't even have

as well as, should these error codes be removed since they can't be obtained?

copper laurel

Didnt stores die

vernal atlas

the store channels did yea

idk if the whole thing went with it

not sure what to name this one, ACCOUNT_VERIFICATION_REQUIRED maybe ?

same for this one, best i came up with is CANNOT_DELETE_COMMUNITY_REQUIRED_CHANNEL but feels a bit too long

oak quail

the store channels did yea
@vernal atlas no

store channels still exist lol

only the centralized store and nitro games died

vernal atlas

ah

tacit crypt

but feels a bit too long

RESTPostOAuth2AccessTokenWithBotAndGuildsAndWebhookIncomingScopeResult

CannotDeleteChannelRequiredForCommunityGuilds

vernal atlas

PepeLaugh

vagrant holly
slate nacelle

Oh right, forgot about that

sick merlin

<Guild>.members.fetch() doesn't seem to work anymore with the new intent requirements ( without guild members ), but the error doesn't seem straightforward and the error page in the guide doesn't mention the intent is needed

vagrant holly

You can't fetch members without the members intent, I feel that's pretty obvious, no?

slate nacelle

The error is actually an internal timeout, Discord (probably) just ignores the request.

Since there is no way to always know what intents are present (read: you can omit them on gw6, what we use), we couldn't reliably throw an error here.
Not until v13, then we surely can.

vernal atlas

couldn't you check <Client>.options.intents ?

oh wait- this is for existing v12

vivid field

since WebSocketManager is a custom implementation, where is the ws package used?

slate nacelle

/src/WebSocket.js, which then is used in /src/client/voice/networking/VoiceWebSocket.js, and src/client/WebSocket/WebSocketShard.js.

vivid field

oh right, i forgot that each shard has its own ws connection, thanks

vagrant holly

Just as an aside, 12.4.x had some nice memory usage improvements (I was running 12.3.1 w/ my message edit limit patched in). No idea what else changed w.r.t to mem usage, but something seems to have done.
@vagrant holly Just to follow up on this, definitely looks like a leak got fixed. No idea what or where, but since updating d.js my memory usage no longer creeps.

tacit crypt

are you sure it wasn't the message edits?

vagrant holly

Nah I was on a hotfix branch with my PR merged before

Cause that was causing me to oom

vivid field

why does collection allow a thisArg? what's the difference between collection.find(fn, myCoolObject) and collection.find(fn.bind(myCoolObject))?

merry python

arrow functions can't be bound, the same parameter exists on standard js functions

vivid field

but neither works for arrow functions meguFace

keen cosmos

consistency with array methods?

solemn oyster

^

real jetty

why do we have seemingly useless properties such as <Guild>.ownerID even though we have <Guild>.owner? Is this just for ease of use or is there a valid reason? I understand Discord supplies only the owner_id and DJS works out the owner's guildmember object itself but would it not make more sense to have one or the other instead of both?

i feel like just having ownerID would be sufficient

copper laurel

Guild#owner is a getter for this.members.cache.get(this.ownerID)

real jetty

yes i've seen which is why i kind've see it as useless, the user could simply get the owner with the ID instead of needing two properties for this

copper laurel

They could, but thats also the purpose of getters

I could make the same argument that message.channel shouldnt exist and we should just have message.channelID

real jetty

true, but if you make that argument:
we could just have <Guild>.owner instead of <Guild>.ownerID

copper laurel

But GuildMembers are not always cached, making ownerID the valid property here

real jetty

ah

yeah

alright thanks for clearing that up

copper laurel

Which is why <Guild>.owner can be null (getters return null if they cant resolve the data) and thats when the user needs to fetch via ownerID

This issue became significantly more present due to intents

frank turret

Has the possibility of moving convenience getters like this to their own package in next been considered? Recall someone mentioning it in the past

real jetty

i feel like removing these types of features has been discussed

thats why i brought it up

and yes intents seem to complicate this sort of thing, guess that adds more reason for the getter

copper laurel

@frank turret their own package? why?

frank turret

Well that was an example of some method of separating them so the whole conversation of "why do we have X convenience method when a person can just do Y”

real jetty

yeah i guess we have to find a middle ground
on one hand some developers don't need these methods at all since they know what they are doing (more or less)
on the other changes like this would break lots of people's code, and new developers may find it difficult to wrap their head around the change

although in my opinion if we're going to remove/change conveniences like this we should do it all at once in a major release to get it over with

copper laurel

Removal of a property is semver major so yes

real jetty

yeah ik, but as in remove all the convenience methods at once, not just remove one or two
that would just confuse people more

tacit crypt

I mean.. considering Discord seems to point at serverless and go: this please; I think we should replace a bunch of getters with fetch<X>

Er, stateless

Close enough

copper laurel

Conceptually agreed, but I feel like there are already other solutions for that and discord.js really is the caching model

raven juniper

I believe what nico is referring to is removing stuff like GuildChannel#viewable, the basic permission checks that the user can very easily implement

fading slate

How are message replies represented in the API/library?

unique axle

?replies

rich iglooBOT

Discord.js will support inline replies in v13, replacing Message#reply with the new functionality.
If you'd like to try out the feature ahead of time (in a server with it enabled), please install and test this PR: https://github.com/discordjs/discord.js/pull/4874
You can e.g. use NPM to do this: npm i discordjs/discord.js#pull/4874/head (this requires git to be added to PATH)

fading slate

Okie

copper laurel

Discord reused the same property crossposts do, so we already support that

fading slate

Oh, Okie 👍🏻

round mortar

How are we looking on d.js v13 (specifically the REST package)?

unique axle

see above

old kelp

isn't Guild#voice an inferior shortcut for Guild#me#voice? I'm not sure if that's any better to obtain the VoiceState from the Guild#voiceStates (GuildMember#voice does the same there but with a better approach)

solemn oyster
oak quail

oh, i asked aj something about that and forgot to finish it after he responded, i'll do it now

ok there

lofty birch

Because I see the PR was merged yet it still happened

lofty birch

On 12.5 too

tawny warren

Why can't inline replies be added to v12?

old kelp
snow crypt

they can, but they shouldn't

that can cause confusion

currently message.reply sends a message with a mention, but the name reply suggests that it is the discord reply feature, and calling it discordReply or anything similar is just- no

copper laurel

My PR was Message#inlineReply at one point, and Souji had done one to remove Message#reply in v13

But we decided just to merge the two and make it a v13 release

unique temple

hi, after the discord.js v13 library exits, will i be able to use discord.js v12 without bot verification?

sorry, my english is not perfect

slate nacelle

Bot verification is pretty much unrelated to libraries, as long as v12 is working, you will be able to use it. (Whether its recommend is another thing though)

slate nacelle
unique temple

A friend told me that I will be able to use the discord.js v12 library until June, after June I need a verified bot to be able to use it

@real jetty tłumacz sie

slate nacelle

No, I am not aware of such a thing, especially since verification is only available after your bot is in 76 guilds.

real jetty

what

unique temple

I hope discord.js v12 can still be used after June.

oak quail

@unique temple discord.js v12 will stop working sometime next year

it has nothing to do with whether your bot is verified or not

but updating to v13 will probably not be hard

tacit crypt

How will v12 break? Aren't you confusing it with v11?

Oh, wait, api v7

Nvm me

lofty birch

I'll try something out in a few hours that could find a cause, but can't atm

V4 said that the original issue was caused by a process aborting, not exiting. And the manager would then not restart that

vagrant holly

^ just tested on 12.4.1 & 12.5.0, a shard respawns on 12.5.0 when process.exit() is called where it wouldn't on 12.4.1, so I'm confident in saying its fixrd

lofty birch

One of my shards died and was off for 15 hours this morning until I restarted the manager

real jetty

Hmm, quick question, why is publicUpdatesChannel read-only but publicUpdatesChannelID not?

vivid field

because publicUpdatesChannelID is a property sent from discord, whereas publicUpdatesChannel is just a getter which returns the channel with that id from the channel cache

real jetty

Ahh, I see. Thank you

So then i guess that answers the question I was about to ask, "why is it publicUpdatesChannel and not modUpdatesChannel or something", because that's the property name sent from Discord?

vivid field

correct, most stuff is named according to the discord api

vagrant holly
lofty birch

and I fetch those values via bot.shard.broadcastEval('this.uptime ? this.guilds.cache.size : 0')

vagrant holly

so the shard is alive then

broadcastEval will throw if the shard is dead

lofty birch

but apparently not logged in

vagrant holly

okay so this is an unrelated bug to before

So you never see a death event in your logs?

lofty birch

I wasn't aware there was a death event

on manager there's only shardCreate

and there's obviously not one on client

vagrant holly

Yeah so on shardCreate you then bind to shard events

shard death will trigger a respawn

lofty birch

I do listen to that, no ready events were logged in that time

vagrant holly

I'm confused, do you listen for shard death?

lofty birch

only shard ready

vagrant holly

Ah right

I'd suggesting listening for shard death/disconnect in your manager

and seeing what you get back

this feels more like some sort of disconnect issue rather that outright shard death

lofty birch

Oh I didn't notice those extra events on the shard

yeah, will do

oh I already listen for disconnect actually (which I've not had since my logs began)

just not death

Added that now. Other than that is there anything you can recommend?

vagrant holly

Honestly not, never seen this before in my experience

drifting knot

right now the methods for banning and kicking members/users accept options in 2 ways:

// kicking:
member.kick(reason?: string)
// banning:
member.ban(options?: { reason?: string, days?: number });

would it make sense to have the kick methods also accept an object for options, so that it's easier to reuse code to ban/kick someone and pass a reason?

lofty beacon

not in my opinion

frank turret

Well, that would make sense only if the object would take more than one param. Kick only has one param you can pass in

lofty beacon

the way it is is fine imo

lofty beacon
real jetty

i addressed this in PR #4718 however that was considered semver major, however changes with partials mean this would now be considered semver patch, correct?

tawny warren
old kelp

you could read a few messages below that and as well read the PR description

sullen bane

Have there been any discussions on the github about inlining the documentation in the type definitions? I looked but could not find any.

vagrant holly

Disconnecting the docs from the source seems like an odd move imo. When I’m working on the lib source, having all the jsdoc right there is rather helpful

oak quail

tbh it would be useful to have it in the types

having it in the source is nice for lib devs but having it in the types is much more useful for users

sullen bane

Agreed, but we could probably rig up a system to mirror it.

Like if we include docs links for each method in the typing, we could probably use a small script to populate the comments in the typing

oak quail

ig

at that point, typings could probably just all be handled automatically?

sullen bane

Not quite. The typeings in TS are much more precise than what is in the jsdoc.

vernal atlas

yeah generating the types from the jsdoc would either make the typings inaccurate or the jsdoc super wack

copper laurel

Im not sure what you're looking to achieve here?

Having the docs appear with Intellisense?

oak quail
copper laurel

Was decided against because we don't want to maintain two sets of docs

sullen bane

Worth looking into if it can be automated.

tacit crypt

If it wasn't said before, that issue will be solved with -next owo

clever crypt

correct me if i'm wrong but, can breaking changes be performed to the voice aspect of discord.js since it depends on @discordjs/opus which doesn't have a defined public api (basically it doesn't have a v1 yet)

wild flax

No

sullen bane

Semantic versioning is a human thing, there are no technicalities with it, even if you could do that -- it would be very rude.

empty viper

What means typing count in this method ?

vagrant holly

Internally there is counter, while the counter is greater than one, typing will be sent to the channel

so stopTyping needs to be called counter times to actually stop the typing

(e.g. if you use startTyping when a user runs a command, and use stopTyping when the command executes, the counter allows for two commands to be run at the same time in the channel and not break that typing effect)

So when you call startTyping, you can increment that counter by a custom count if you want. So you could pass 2, and then you'd need to call stopTyping twice to actually stop that typing (or use the force flag)

empty viper

Okay I see I see, thanks for explanation, and it should be more precise in the description in the doc !

vagrant holly

Imo its a pretty weird edge case to need to use the count arg, so having more detailed docs might confuse folks into thinking they might actually need to use it more usually

But feel free to create an issue if you want, to log the request for the docs to be improved 👍

limber thistle

are djs' caching issues gonna be addressed in future versions

like the reason djs-light exists

tacit crypt

the only thing that lib does is disable caching. It's not an issue that we do what Discord used to tell everyone (aka cache stuff). If the question would be like are there plans on changing how caching works in the future, then the answer can become, say, maybe, or depends or idk

vagrant holly

If you have specific caches that are causing you issues, please do say, I'm always looking to add settings to reduce those

But you can just manually sweep most things yourself and D.js will keep working for the most part, esp. if you've got partials enabled

tacit crypt

^

limber thistle

right thanks

its just its not very feasible to run djs on large bots, atleast for me with subpar hosting

tacit crypt

Sure there is..

vagrant holly

What do you consider "big"?

tacit crypt

it's called sweeping

limber thistle

a server bot i ran in heroku on one server with 220k members couldnt handle it

it had intents disabled

my other bot reaching 600 servers runs smoothly rn so thats nice

tacit crypt

In highlight, I sweep a bunch of things and the bot works just fine shrug, with little memory too! Course, if there's any cache you notice that eats up a LOT of ram compared to the rest, let us know, preferably with heap dumps

limber thistle

right

vagrant holly

Manually sweeping members + users will probably help you in that situation (plus ensuring you're not fetching all on boot etc.)

limber thistle

yeah im not, currently i upscaled the hosts to fix it

tacit crypt

using just intents you NEED is also a huge improvement

limber thistle

i turned off all privileged intents too

tacit crypt

so then your bot did nothing?

vagrant holly

I might look at adding some sort of options into D.js for sweeping users/members, I wonder what folks would think of that

limber thistle

yeah that would be nice too

tacit crypt

Eh, I'd rather at that point we get some configurable caches instead of piece by piece

limber thistle

my bot worked when i increased its allocated ram

configurable cache is also nice

tacit crypt

If your intents array was empty, then your bot did nothing

if you didn't specify intents, it means you just get all events

frank turret

does d.js-next plan on allowing us to use our own caching structures? Like if i wanted to use redis for the cache

limber thistle

sorry i mean privileged intents

tacit crypt

GUILD_MEMBERS and GUILD_PRESENCES are.. shrug

depending on each guild, TYPING_START is nightmare, VOICE_STATE_UPDATES, etc

limber thistle

ok

i dont need any voice stuff

ill look into that thanks mate

warm crater

Discord.js should go back to what it was before datastores and managers, the high level API it exposed made sense, it just needed to be refactored. DataStores broke every basic rule of software design, Managers tried to correct it, but left us with inconsistencies in the API. To me, there doesn't seem to be a good argument for something like channel.messages.fetch over channel.fetchMessage. Just because you replace multiple word methods with multiple single-word object fields doesn't make it better or "more object-oriented".

Slight tangent, but most of the problems caused by bad design could be somewhat mitigated if the library allowed for easy dependency injection through constructors (or some kind of IoC container). This would do away with things like cacheType, cacheOptions, Structures.extend etc which are all ad hoc fixes to the current dependency injection situation.

Just some thoughts I've had for a while...

wild flax

Short answer: yes.
Long answer: Not quite, no.

You said yourself that Managers adjusted for the shortcomings that DataStores have, but on the same page you say theres no good argument over them? That seems to contradict yourself.

Overall you are not wrong when it comes to dependency injection, but this is far from a "simple" change. It takes quite some time to implement that or even do it correctly.
And even if we do it, there would also be no "going back" to the old structure. With correctly applied DI it would look somewhat similar to what we have now still, just not as inconsistent.

warm crater

I'm saying there is no good argument for the current API (that uses managers), over the API that v11 exposed (which didn't have managers or datastores). DataStores tried to separate the responsibilities of the old ClientDataManager, but they also changed the API (e.g. channel.messages.fetch) without good reasoning, this side effect resulted in managers to once again separate cache and Discord API calls, v11 already had this separation. Now we are left with things like channel.messages.cache, channel.messages.fetch alongside channel.fetchInvites, which is inconsistent and clunky. Whereas if we stuck with the v11 style we would have channel.messages for the cache, and we would have consistency with API calls like channel.fetchMessage, channel.fetchInvites etc.

Managers adjusted for the some of the shortcomings of DataStores, but not all of them.

wild flax

Yeah there is enough good arguments. And all of them are listed in the issue that eventually led to managers. If you just completely overlook those for the sake of your "more words bad" argument then I don't know how I else I can help you understand.
The more "oop-ey" approach was not the motivation at all here.

vagrant holly

Imo, $0.02, having methods that affect cache inside a manager instead of the parent model is cleaner. Yes, it leads to some inconsistency with model methods that don't have a cache, but I think it provides a clear separation between methods touching cache and those that don't.

wild flax

Managers adjusted for the some of the shortcomings of DataStores, but not all of them.
And that exactly was the whole point. There is not really any "fixing it all" with the current iterations without a complete rewrite of how the system works. Which would be immensely more time consuming on top of all possible bugs this could introduce. So we are not only talking a new major version increase but also one that has a very high risk associated to it.

I don't know how you write software, whether that be hobby-wise or professionally, but since this is mostly a non-profit open source project you can or rather don't want to take such risks if you are aware that it could lead to hundreds or more hours you need to put into "fixing" while you already pushed it to users out there to use but its in a complete broken state.

There is a huge component that influences all decisions we make that isnt just "writing code" at the end of the day here. Writing code is maybe 30-40% if at all.

warm crater

I did read the original issue, it lists DI as one of its benefits, but there are other ways of doing DI that would have worked with the old API, its not unique to managers. It also lists separating cache and API as a benefit, but reverting also has this effect. It also says that one of the main reasons is that the data store API is more fluent than the v11 API (the more words bad argument), which is what I disagree with.

I think the risk of removing Managers would be similar to the risk of adding DataStores, which admittedly took a long time if I recall but it did happen eventually, so I would say its possible without a complete rewrite. DI was more of an afterthought I had, that would definitely need a complete re-write (probably in TypeScript) for it to be solid and not hacky.

wild flax

Feel free to open a PR if you think its the same risk and effort.

The reason Mangers took that long was simply because no one worked on it for a long time, not because it actually did take a long time.

copper laurel

It's also worth pointing out here that DataStores were never released. They were never in v11, and they weren't in 12.0.0

limber thistle

Thoughts on having separate partial events that you could specify in clientOptions? ,
partials will only fire in the events specified in that option so that we dont need to do if(class.partial ) return for all the other events if we decide to use partials

vernal atlas

eh

real jetty

do you mean different events for partial data or selecting which events the client would emit if the data provided is partial

limber thistle

same event, if you enable partialEvents, it only fires partialData if that event is enabled

if you dont give any partial events it doesnt affect anything

say I only want partial data for the reaction event

right now i have to go to each event and add if(partial) return

and use it only in reactionEvent

I can submit a pr if this is a good idea

oak quail

could use client.ws.on to get the raw MESSAGE_REACTION_ADD event

limber thistle

guess i could

unique axle
oak quail

is there any chance of bringing back disabledEvents?

glad sparrow

Why was it removed tho

ornate topaz

cause intents were created

it has pretty much same effect, apart from the fact that intents are groups and are never sent to you, instead of just being dropped by lib upon receiving

oak quail

that "apart from" matters tho

https://github.com/discordjs/discord.js/pull/3159

This gives a public alternative to the private raw event. (The raw event isn't removed, for the purposes of library testing.) This gives devs the ability to work with d.js in a cacheless way without relying on the private raw event.

Like raw, these event's aren't disabled with ClientOptions#disabledEvents (which isn't very aptly named, since it only disables the internal d.js handlers).
however they are disabled by intents

also you dont get per-event control with intents

glad sparrow
prime prairie

When I do member.premiumSinceTimestamp, I sometimes get 0, or I get null depending on the user (That aren't boosting), is there a reason why these are sometimes different?

Thats a log of 2 users

vagrant holly

When the messageReactionAdd partials fix was done, did that also update/fix messageReactionRemove?

From my super limited test, if I start a bot up and remove a reaction, the event isn't fired

slate nacelle
vagrant holly

Huh, there is a user id there?

I wonder if it fires if I enable user partials

slate nacelle

Yeah, then it should.

frank turret

Any specific reason @discordjs/docgen has eslint as a main dep? Along with using Tsubaki just for a simple Promisify that could be done with Util.Promisify? The engine field in the package.json intends for docgen to be used with Node >=8.0.0 so Tsubaki is kind of useless here. Is it worth opening a PR over just to remove those? Asking cause I intend to use docgen for a personal project. I'mma go ahead and make a PR anyways OMEGAlul

empty viper

Shen does discord.js-next will start to be programmed ?

copper laurel

Whenever the maintainers feel like working on it

vernal atlas

when can APIMessage#target be a User or a GuildMember?

warped crater

if I had to guess, DM channels result in it being a user for a start

vernal atlas

but they are DMChannel instances ?

warped crater

huh

vernal atlas
warped crater

ah, it may be because both DMChannel and User implement TextBasedChannel, sug

vernal atlas

hm

warped crater

that's still weird though

vernal atlas

i guess if you create your own APIMessage instance maybe

thats like the only place i can see it being a user/guildmember

warped crater

looks like a legacy of the past tbh

vernal atlas

possibly

real jetty

Im curious, when d.js sends a request to the api and it comes back, does d.js translate the received properties into javascript conventions (like camelCase for example)? Or does it come back already camelCase so there isn't much to do?

i could actually probably check the source couldnt i

interesting, i'm assuming this var instead of let is intentional? smart

real jetty

So looking through at least the Message structure, it seems that if Discord were to add a property to the message object returned by the API, it wouldn't be seen by the end user (the one using d.js). Why is that?

vagrant holly

The library isn't really just an API wrapper, it provides proper custom interfaces, so it doesn't just expose API data automagically

warm crater

You would also have to manually update the typings to support it anyway.

warm crater

And as v4 was saying, if you use some JavaScript magic and dynamically rename snake_case properties to camelCase then you lose the ability to choose what kind of abstraction you want to expose to developers. This would make the library very unstable because some people would adopt the new property and now you can't change how its exposed without breaking everything.

copper laurel
  • yes we do translate snake_case to camelCase
  • yes the structures are custom and don't expose new properties
  • unmodified Discord event packets can be listened to via client.ws.on("MESSAGE", message => etc
long perch

discord.js-commando should have an implemented property that allows the automatic cancel response to be changed or disabled.

empty viper

Is there a version of this library for Deno ?

raven juniper

no, only node

empty viper

Oh...

vernal atlas

there seems to be a slight issue with the typings for the send method on PartialTextBasedChannelFields, passing something that can be of multiple types results in an error
eg:

const options: string | MessageOptions = ...;
channel.send(options);```
this is perfectly valid code, however results in an error with TS, because none of the overloads technically accept this

i think *more* overloads need to be added here, but it seems like its getting very bulky there are already alot of overloads
real jetty
unique axle
unique axle
vernal atlas

@tacit crypt what precisely do i need to update in my v8 PR for the ratelimits? i did a test and on v7 the API sends 5 as the x-ratelimit-reset-after, but on v8 sends 5.000, so i would assume that rather than 4 for 4 seconds being returned now 4.567 would be returned for 4 seconds and 567 milliseconds? the number is already multiplied though so would be 4567 - or am i just being stupid and missing something

tacit crypt

retryAfter is in seconds

vernal atlas

oh

oh i see now i think

yeah i was being dum dum

tacit crypt
vernal atlas

applied change

ruby terrace

I'm currently working to implement interactions (slash commands being the only interaction for now), based on the last pinned message, should I hold off on my PR?

copper laurel

Yes, probably. The API docs PR is still marked as a WIP with comments off. Given that even Discord doesn't want it open for discussion yet, probably best to hold off

ruby terrace

Ok thanks, I was going to mark mine as a draft /WIP as well and keep along with the updates, but I'll just wait until they have finalized their docs

tired valve
ruby terrace

Whoops, thank you!

drifting knot
ruby terrace

I don't think we get ETA's on that, but I believe the intent was to have gateway v8 be a part of the v13 release.

ornate topaz

the switch to v8 is the reason there has to be a v13 in the first place

tired valve

& reply() being inline replies is a major change

drifting knot

does djs use GH milestones? would be nice if appropriate PRs were tagged with a v13 milestone

unique axle

due to how we use checkboxes for versioning rather than for progress milestones aren't all that useful
(as milestones accumulate progress based on checkboxes)

drifting knot

oh L

agile blade

Had a question on a comment on a PR I’m working on, can someone clarify for me? It’s on PR #5026

unique axle

if you actually ask the question one might be able to answer

agile blade

Just wanted to make sure I was in the right channel...
There’s a comment asking about the options object of the method, and if it could extend RoleData. I’m not entirely sure what is being asked as a change, I know it’s a documentation related thing but unsure on specifics

lofty birch

I've noticed that when updating a username, guild member update is fired with an identical user object (both of which have the new username). I presume this is unintended behaviour?

unique axle
lofty birch

ah that sucks. any workaround?

unique axle

i'd recommend reading the issue responses

lofty birch

Oh that's interesting, it's apparently caused by presence updates, but I don't use presence intents

It must be from user update arriving shortly before member update then, regardless of presence updates

unique axle

and if you now read the issue comments again you will realise that userUpdate is a artificial event that's not directly emitted from the API, as USER_UPDATE only emits for the client user and instead created based on the data in GUILD_MEMBER_UPDATE (which also emits on underlying User data) as well as PRESENCE_UPDATE (which also control the initial GuildMember and User caches to be filled) payloads.

empty viper

Are managers coming to djs-next ?

ornate topaz
empty viper

Oh...
This should be made in a way that you don't have to put .cache everywhere, like a subclass of Collection that have the methods to fetch or resolve

oak quail

that was DataStores which got replaced with Managers before v12 was officially released

and it is good to have .cache because although it is longer, you know that you are accessing the bot's cache and it will likely not be full

unique axle

to get it out of the way: "i have to put a word more" is in no phase of the design process a consideration we make. as harsh as it might sound: "deal with it."

the clear separation of cache from API access is a concept we will be moving forward with. we have tried the other approach shortly during the development of v12 with "stores" which we have discarded as it caused more issues than it solved. with the rewrite considering modularity (plugging in own cache implementation) the separation becomes even more worthwhile.

in the current version 12.5.1 managers don't seem to be very useful, however there is a PR which will cover this (likely with v13) #4869 and will expose direct API access (so you don't need to unnecssarily fetch things before operating of them if you have the required data), again putting more value in a clear separation.

a general word about discord.js-next : the rewrite is not a sharp picture yet. we notice issues with the current approaches which can not be solved without a major redesign which we consider for the rewrite. we are aware that mangers in their current iteration are not optimal and have expressed that internally as well as externally, howver we do believe that they are better than the v11 approach of having them on the structure they belong to or the (discarded) v12 in-dev approach of mashing cache and api access together (which seems to be what you propose here)

snow crypt

would the way to fix this be removing ```yaml
- run: git checkout HEAD^2
if: ${{ github.event_name == 'pull_request' }}

ruby terrace

This is a very strange and unlikely edge case, but currently if you want to edit a message and change suppress embeds (basically if you are editing in an embed, and suppress was currently enabled) the API gets two calls unless the user knows to add the undocumented flags parameter as an option.
Would it be a good idea to clean this up and add a suppressEmbeds option to MessageEditOptions and completely remove suppresEmbeds since it is just a helper method?

oak quail

@ruby terrace I don't think suppressEmbeds should be removed since there are tons of set__ helper methods which do basically wrap edit

copper laurel

edit should support the flags though if it doesn't already, I agree with that much

ruby terrace

edit does support flags, it just wasn't in the MessageEditOptions typedef, and only suppressEmbeds passed it

ruby terrace
vale moss
wild flax

Devils advocate:
If discord ever decides to again, change something in their naming, we don't need people to change it in all their code because discord decided to rename something or whatever (thought further down the line you prolly still want to rename it, so take that with a grain of salt)

2nd, people already typo those permissions left and right, ive seen stuff from "SNED_MESSAGE" to "GILD_MEMER"

This assures this surely wont happen

vale moss

So you dont want users to change anything, but change lib api extremely?

However, i think lib should give users choice what to use and how to

merry python

2nd, people already typo those permissions left and right, ive seen stuff from "SNED_MESSAGE" to "GILD_MEMER"
this only works in TS where the compiler can actually tell you whether or not you're using a valid value, if you typo Permissions.FLAGS.SNED_MESSAGE in JS it's no better than typoing "SNED_MESSAGE". But it's better than nothing

wild flax

JS auto-completion should be good enough to pick this up

To at least list you all the possible values

wild flax
vale moss

Agree, extremely do

wild flax

This isn't just a wrapper after all, its a whole library

A wrapper would just expose and let you do all the other stuff, no caching, no nothing at the end of the day

We wouldn't then even need the abstraction for events we have, since you could just listen to .on('MESSAGE_CREATE') yourself for example

Its a tough choice sometimes

vale moss

The Best choice is change nothing

wild flax

Thats subjective here

Because as much as you say that, I could argue the best choice is to be as restrictive as possible

vale moss

Lib must be restrictive from the beginning

wild flax

I can also change

Thats the purpose of major changes

tender field

Hi, why is discord.js not using a transpiler (such as babel)?
It seems the "main" reason to drop support of node 12 is to benefit of the new ES2020 features, but why not just use babel? It will also allow the use of the "import" notation (which I know is only syntactic sugar), and it will not even be, i think, a breaking change, will it ?

frank turret

is that really necessary to do that when djs-next is being rewritten in typescript and can be transpiled down to a lower es version?

tender field

no ofc, but why wasn't it use from the beginning? (sorry for the english btw)

tender field

i mean it's not a problem for me, i'm already on 14.5.1, but i'm just curious 😅

undone ravine
tender field

okay

low minnow
ruby terrace
low minnow

Alright, ty

empty viper

Does this will come to djs v12 or v-next ?