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

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

Closed
CarmJos opened this issue Nov 26, 2023 · 7 comments · Fixed by #331
Closed

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

CarmJos opened this issue Nov 26, 2023 · 7 comments · Fixed by #331
Labels

Comments

@CarmJos
Copy link
Contributor

CarmJos commented Nov 26, 2023

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 CarmJos changed the title Support generic "Argument Registery" to support multi-plugin project. Support generic "Argument Registery" to facilitate multi-plugin development. Nov 26, 2023
@CarmJos
Copy link
Contributor Author

CarmJos commented Nov 26, 2023

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
Copy link
Owner

Rollczi commented Nov 26, 2023

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

@Rollczi Rollczi added help wanted Extra attention is needed refactor architecture labels Nov 26, 2023
@CarmJos
Copy link
Contributor Author

CarmJos commented Nov 27, 2023

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
Copy link
Owner

Rollczi commented Dec 10, 2023

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
Copy link
Contributor Author

CarmJos commented Dec 11, 2023

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
Copy link
Owner

Rollczi commented Dec 11, 2023

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
Copy link
Contributor Author

CarmJos commented Dec 16, 2023

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.

Rollczi added a commit that referenced this issue Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants