Update DefaultShardManagerBuilder.java

[Horstexplorer]

Changed access modifier to protected so we can extend the class to add / override some functionality

Pull Request Etiquette

• I have checked the PRs for upcoming features/bug fixes.
• I have read the contributing guidelines.

Changes

• Internal code
• Library interface (affecting end-user code)
• Documentation
• Other: _____

Closes Issue: NaN

Description

This pull request changes the access modifier of the DefaultShardManagerBuilder to protected to allow us to extend the class.

[Andre601]

Is there a particular use case that would require the extending of the DefaultShardManagerBuilder?

[arynxd]

A custom builder

[MinnDevelopment]

I would prefer to add a second constructor which accepts EnumSet<GatewayIntent> instead of an int. This constructor can be used to set invalid intents which should not be allowed even by subclasses.

[MinnDevelopment]

If you intend to let classes extend this, then methods like applyDefault and applyLight should also be changed to protected.

[MinnDevelopment]

Can you give me an example of how you would extend this class?

[Horstexplorer]

I am trying to figure out how to prepare a ShardManager which allows to manually start individual shards (0 to n of m) without starting up all of them.
Currently calling build(false) will move the login of all shards to a point where we either call login() or start(...) which then logs in all shards in order.
I probably missed something but so far I have had no success with it, but for testing purposes it seemed to be easier to modify a working builder than to copy and implement a custom one.

[MinnDevelopment]

I mean, give me an actual example implementation of a subclass.

[Horstexplorer]

Oh my bad.
I had something like this in mind due to my things above. However there are several possible conflicts with return values and those static create methods which might make extending it unviable.

public class CustomShardManagerBuilder extends DefaultShardManagerBuilder{

    protected CustomShardManagerBuilder(@Nullable String token, EnumSet<GatewayIntent> intents)
    {
        super(token, intents);
    }

    public static CustomShardManagerBuilder createCustom(String token){
        return new CustomShardManagerBuilder(token, ...).applyCustom();
    }

    protected CustomShardManagerBuilder applyCustom(){
        return this.setMemberCachePolicy(...)
                   .setChunkingFilter(...)
                   .disableCache(...)
                   .setLargeThreshold(...);
    }

    ...

    @Override
    public ShardManager build(boolean login) throws LoginException, IllegalArgumentException
    {
        checkIntents();
        boolean useShutdownNow = shardingFlags.contains(ShardingConfigFlag.SHUTDOWN_NOW);
        final ShardingConfig shardingConfig = new ShardingConfig(shardsTotal, useShutdownNow, intents, memberCachePolicy);
        final EventConfig eventConfig = new EventConfig(eventManagerProvider);
        listeners.forEach(eventConfig::addEventListener);
        listenerProviders.forEach(eventConfig::addEventListenerProvider);
        final PresenceProviderConfig presenceConfig = new PresenceProviderConfig();
        presenceConfig.setActivityProvider(activityProvider);
        presenceConfig.setStatusProvider(statusProvider);
        presenceConfig.setIdleProvider(idleProvider);
        final ThreadingProviderConfig threadingConfig = new ThreadingProviderConfig(rateLimitPoolProvider, gatewayPoolProvider, callbackPoolProvider, eventPoolProvider, audioPoolProvider, threadFactory);
        final ShardingSessionConfig sessionConfig = new ShardingSessionConfig(sessionController, voiceDispatchInterceptor, httpClient, httpClientBuilder, wsFactory, audioSendFactory, flags, shardingFlags, maxReconnectDelay, largeThreshold);
        final ShardingMetaConfig metaConfig = new ShardingMetaConfig(maxBufferSize, contextProvider, cacheFlags, flags, compression, encoding);

        final CustomShardManager manager = new CustomShardManager(this.token, this.shards, shardingConfig, eventConfig, presenceConfig, threadingConfig, sessionConfig, metaConfig, chunkingFilter);

        if (login)
             manager.login();

        return manager;
    }
}

There probably is an easier solution to this.

[Horstexplorer]

Perhaps it would be better to ask for the feature directly instead of making non optimal extensions of the builder to try things out.
Would it be possible to allow us to just start an individual shard instead of all selected ones when calling start() for a specific shardId? With the current behaviour all of the selected shards will start independently from the shardId which makes organizing the start of shards more difficult over multiple jvms.

[Horstexplorer]

A possible workaround for this behaviour would be to tell jda only about the first shardid you would like to run and then call start(...) for all shardids beginning with the first one.

[MinnDevelopment]

I think it would be fine to update the shard manager to properly cater to the needs of dynamic clustering. A potential for empty shard managers that are dynamically populated is definitely there and could probably be implemented with no breaking changes. If someone wants to do those changes and make a separate pull request, that would be ideal. Your thoughts?

[Horstexplorer]

That sounds to be the right solution.

[DV8FromTheWorld]

@Horstexplorer Are you planning to open a new PR for that functionality or create an Feature Request issue?

[Horstexplorer]

Sure, I just did not manage to find time to do it properly yet.
Will do so now