Rollczi / LiteCommands

☄️ LiteCommands - Command framework for Velocity, Bukkit, Paper, BungeeCord, Minestom, Sponge, Fabric, JDA and future implementations.
https://docs.rollczi.dev/
Apache License 2.0
146 stars 21 forks source link

Support generic "Argument Registery" to facilitate multi-plugin development. #306

Closed CarmJos closed 11 months ago

CarmJos commented 12 months ago

I'm using this library in a large server project, and I expect to be able to maintain a public "Argument Registry" so that when a plugin registers a new Argument, it can be updated to all plugins/commands that use this public Registry, thereby avoiding the need to register an Argument each time and improve Code unity and atomicity.


For example, a main project called "xxCore" provided User.class and Foobar.class.

If i want to use those arguments in other plugin like xxLobby, i must re-register those arguments and resolvers to support it.

And then, a new project called xxLobbyTime requires a LobbyUser.class argument from xxLobby...

Many arguments are reqiured to be registered if every plugin use the single, finalized, private ParserRegistery and SuggestRegistry.

I tried some hacky ways like:

protected void applyRegistry(LiteCommandsBuilder<SENDER, SETTINGS, ?> builder) {
        builder.invalidUsage(createUsageHandler()).schematicGenerator(formatter);

        // Change registry through reflection
        try {
            Field messageRegistryField = builder.getClass().getDeclaredField("messageRegistry");
            messageRegistryField.setAccessible(true);
            messageRegistryField.set(builder, this.messageRegistry);

            Field parserRegistryField = builder.getClass().getDeclaredField("parserRegistry");
            parserRegistryField.setAccessible(true);
            parserRegistryField.set(builder, this.parserRegistry);

            Field suggesterRegistryField = builder.getClass().getDeclaredField("suggesterRegistry");
            suggesterRegistryField.setAccessible(true);
            suggesterRegistryField.set(builder, this.suggesterRegistry);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

And recode all register functions:

    @Override
    public <T> void registerArgument(Class<T> type, ArgumentResolver<SENDER, T> resolver) {
        registerArgument(type, ArgumentKey.of(), resolver);
    }

    public <T> void registerArgument(Class<T> type, ArgumentKey key, ArgumentResolver<SENDER, T> resolver) {
        this.parserRegistry.registerParser(type, key, resolver);
        this.suggesterRegistry.registerSuggester(type, key, resolver);
    }

    public <IN, T, RESOLVER extends Parser<SENDER, IN, T> & Suggester<SENDER, T>>
    void registerArgument(Class<T> type, RESOLVER resolver) {
        this.parserRegistry.registerParser(type, ArgumentKey.of(), resolver);
        this.suggesterRegistry.registerSuggester(type, ArgumentKey.of(), resolver);
    }

    public <IN, T, ARGUMENT extends Argument<T>, RESOLVER extends TypedParser<SENDER, IN, T, ARGUMENT> & Suggester<SENDER, T>>
    void registerArgument(Class<T> type, ArgumentKey key, RESOLVER resolver) {
        this.parserRegistry.registerParser(type, key.withNamespace(resolver.getArgumentType()), resolver);
        this.suggesterRegistry.registerSuggester(type, key.withNamespace(resolver.getArgumentType()), resolver);
    }

Then register all arguments by myself (replacing those registry means all default arguments will be cleared).

    protected AbstractCommandsManager() {
        registerArgument(String.class, new StringArgumentResolver<>());
        registerArgument(Boolean.class, new BooleanArgumentResolver<>());
        registerArgument(boolean.class, new BooleanArgumentResolver<>());
        registerArgument(Long.class, NumberArgumentResolver.ofLong());
        registerArgument(long.class, NumberArgumentResolver.ofLong());
        registerArgument(Integer.class, NumberArgumentResolver.ofInteger());
        registerArgument(int.class, NumberArgumentResolver.ofInteger());
        registerArgument(Double.class, NumberArgumentResolver.ofDouble());
        registerArgument(double.class, NumberArgumentResolver.ofDouble());
        registerArgument(Float.class, NumberArgumentResolver.ofFloat());
        registerArgument(float.class, NumberArgumentResolver.ofFloat());
        registerArgument(Byte.class, NumberArgumentResolver.ofByte());
        registerArgument(byte.class, NumberArgumentResolver.ofByte());
        registerArgument(Short.class, NumberArgumentResolver.ofShort());
        registerArgument(short.class, NumberArgumentResolver.ofShort());
        registerArgument(Duration.class, new DurationArgumentResolver<>());
        registerArgument(Period.class, new PeriodArgumentResolver<>());
        registerArgument(Enum.class, new EnumArgumentResolver<>());
        registerArgument(BigInteger.class, new BigIntegerArgumentResolver<>(messageRegistry));
        registerArgument(BigDecimal.class, new BigDecimalArgumentResolver<>(messageRegistry));
        registerArgument(Instant.class, new InstantArgumentResolver<>(messageRegistry));
        registerArgument(LocalDateTime.class, new LocalDateTimeArgumentResolver<>(messageRegistry));

        getParserRegistry().registerParser(String.class, JoinArgument.KEY, new JoinStringArgumentResolver<>());
        registerArgument(boolean.class, FlagArgument.KEY, new FlagArgumentResolver<>());
        registerArgument(Boolean.class, FlagArgument.KEY, new FlagArgumentResolver<>());
    }

Well, this is too painful and ungraceful.

CarmJos commented 12 months ago

In the past two days, I have roughly reviewed this project twice, and I know that it is quite difficult to deconstruct and support this feature.

I wonder if there is a better idea to achieve this requirement?

Rollczi commented 12 months ago

I looked at this, and it's not easy to do... static, copy or setters are not good solutions. I need to think about

CarmJos commented 12 months ago

Is there any possibility that provide a builder method to add external registry for instances? In this way, we can also move the general types into a single, static external registry, instead of registering all of them in final build().

Rollczi commented 11 months ago

Is there any possibility that provide a builder method to add external registry for instances? In this way, we can also move the general types into a single, static external registry, instead of registering all of them in final build().

okey, @CarmJos what do you think about this? #331

CarmJos commented 11 months ago

Is there any possibility that provide a builder method to add external registry for instances? In this way, we can also move the general types into a single, static external registry, instead of registering all of them in final build().

okey, @CarmJos what do you think about this? #331

This pr given a better solution for implementing the applyRegistry(LiteCommandsBuilder<SENDER, SETTINGS, ?> builder) in the topic.

But maybe we also need a registry like "External Public Registery" for other arguments. Because if we just only supplied all commands the same registries, in the build() function, primary arguments will be registery again and again, witch may cause problems.

So let's think about the further solution!

Rollczi commented 11 months ago

But maybe we also need a registry like "External Public Registery" for other arguments.

This is the problem with decentralized object-oriented architecture of litecommands (we assume that litecommands instances are independent of each other), also it is not possible to have two registries (because the parser's search mechanics require viewing the entire register to resolve argument keys, input types, output types and namespaces).

But we can copy all parsers from one to another registry, or you can implement a custom registry (ParserRegistry interface) with additional static fields.

CarmJos commented 11 months ago

But maybe we also need a registry like "External Public Registery" for other arguments.

This is the problem with decentralized object-oriented architecture of litecommands (we assume that litecommands instances are independent of each other), also it is not possible to have two registries (because the parser's search mechanics require viewing the entire register to resolve argument keys, input types, output types and namespaces).

But we can copy all parsers from one to another registry, or you can implement a custom registry (ParserRegistry interface) with additional static fields.

That's okay. It's enought that the hardest part has been solved.