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

Add support for threads #1613

Closed
wants to merge 2 commits into from
Closed

Add support for threads #1613

wants to merge 2 commits into from

Conversation

DV8FromTheWorld
Copy link
Member

@DV8FromTheWorld DV8FromTheWorld commented May 1, 2021

The Threads api is currently in open beta and can only be used in small testing guilds

Currently this encapsulates only the most minimalist pass on being able to construct entities for threads and the happy path of receiving messages in threads.
Only "supports" receiving messages via the GenericMessageEvent and MessageReceivedEvent

This PR will be in a draft state for quite a while and is here for collaboration. Expect incredibly breaking changes as we prepare for threads.

Currently this encapsulates only the most minimalist pass on being able to construct entities for threads and the happy path of recieving messages in threads.
Only "supports" receiving messages via the GenericMessageEvent and MessageReceivedEvent
/**
* TODO docs | https://discord.com/developers/docs/resources/channel#channel-object-channel-types
*/
GUILD_NEWS_THREAD(10, -1,true), //TODO confirm handling of sort bucket.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you can't sort threads so this sort bucket value is irrelevant

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What even is a news thread anyway?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
GUILD_NEWS_THREAD(10, -1,true), //TODO confirm handling of sort bucket.
GUILD_NEWS_THREAD(10, -1, true),

// - getMembers //TODO might not need this as we'll have getThreadMembers()

//TODO pick a better name (or use getParent once we break from GuildChannel interface?)
GuildChannel getParentChannel();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just MessageChannel getAncestor() for now, you can only create threads on message channels anyway.

int getMemberCount();

//TODO | This name is bad. Looking for alternatives.
boolean isSubscribedToThread();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about isJoined()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isJoined sounds, odd to me.

isSubscribed or hasJoined sound better imo

@@ -108,6 +111,11 @@
*/
GUILD_DISCOVERY_GRACE_PERIOD_FINAL_WARNING(17),

/**
* TODO docs | https://discord.com/developers/docs/topics/threads#new-message-types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* TODO docs | https://discord.com/developers/docs/topics/threads#new-message-types
* Message which indicates that a {@link net.dv8tion.jda.api.entities.GuildThread GuildThread} has been created.

Comment on lines +135 to +137
{
throw new IllegalStateException("This message was not sent in a guild");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
throw new IllegalStateException("This message was not sent in a guild");
}
throw new IllegalStateException("This message was not sent in a guild");

public class GuildThreadImpl extends AbstractChannelImpl<GuildThread, GuildThreadImpl> implements GuildThread
{
//TODO this same pattern is used in MemberImpl. Might want to consider centralizing? dunno.
private static final ZoneOffset OFFSET = ZoneOffset.of("+00:00");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could move this to Helpers

@Override
public int getPosition()
{
throw new UnsupportedOperationException("Thread do not have a concept of position");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the client order them? Can we not just use the same ordering?

Copy link
Member Author

@DV8FromTheWorld DV8FromTheWorld Jun 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is either based on join date or creation date. Will need to test, but yeah, it does have an implicit ordering. That being said, it isn't an explicit one and it would change when you leave/join threads, so I'm really on the fence about supporting this.

Really this method only exists on threads because of the interfaces implemented. Providing a position here doesn't make sense especially since position historically implies something that you can modify like with other channels. There's no actionable information for bots here.

@@ -0,0 +1,107 @@
package net.dv8tion.jda.internal.entities;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright

return this;
}

GuildThreadMemberImpl setFlags(long flags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be public?

Comment on lines +711 to +713
{
throw new IllegalStateException("This message was not sent in a guild");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
throw new IllegalStateException("This message was not sent in a guild");
}
throw new IllegalStateException("This message was not sent in a guild");

# Conflicts:
#	src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java
@Nullable
String getArchivingMemberId();

long getArchivingMemberIdLong();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I receive a THREAD_UPDATE event it doesn't include the member who archived the channel. (I've the GUILD_MEMBERS intent)
Am I misunderstanding what this method is supposed to do or?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would get this information regardless of the GUILD_MEMBERS intent. However, this property can be null (for the String version) or 0 (for the long version) when the thread was auto-archived by Discord based on inactivity duration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
  "op" : 0,
  "s" : 8,
  "t" : "THREAD_UPDATE",
  "d" : {
    "last_message_id" : "860587045402968105",
    "audience" : {
      "parent_id" : "859388924278734868",
      "channel_type" : 11
    },
    "parent_id" : "859388924278734868",
    "owner_id" : "823709352509833256",
    "name" : "wa",
    "guild_id" : "859165430752870400",
    "thread_metadata" : {
      "archived" : true,
      "archive_timestamp" : "2021-07-02T18:25:43.757579+00:00",
      "locked" : true,
      "auto_archive_duration" : 1440
    },
    "id" : "859390106968129587",
    "type" : 11,
    "member_count" : 2,
    "message_count" : 8
  }

When manually archiving a thread, am I doing something wrong or missing something? (using latest features/threads branch)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DV8FromTheWorld I'm probably doing something wrong, but just bumping it to make sure.

@rainbowdashlabs
Copy link

Is there a checklist whats missing here?
I really heavily rely on this PR and I would offer my help If someone is willing to provide some minimal checklist of whats still need to be done here.

@Artuto
Copy link
Contributor

Artuto commented Sep 3, 2021

Are there any updates on this? Discord is forcefulling enabling threads on all guilds on 9/9 and JDA still doesn't support them (which is bad for moderation and logging bots).

@Tais993
Copy link
Contributor

Tais993 commented Sep 3, 2021

Threads require changes to the channel hierarchy to be properly implemented, so they'll be JDA 5
For updates I'd say join the Discord and check the (archived) threads within the libdev channel.
Also, check the v5 branch.

@DV8FromTheWorld
Copy link
Member Author

DV8FromTheWorld commented Sep 8, 2021

This PR isn't what we're going to move forward with for Threads in JDA.
Threads will be reimplemented on top of the changes made in the v5 branch.

I'm hoping to publish a list of "what needs to happen to get to Threads" soon.

We've been in a transitionary period for the project, moving from having a single core developer to trying to move to more community involvement, but that comes with some bumps along the way. Threads have not been forgotten.

@DV8FromTheWorld
Copy link
Member Author

JDA v5.0.0-alpha.1 is released and has support for threads.
https://github.com/DV8FromTheWorld/JDA/releases/tag/v5.0.0-alpha.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants