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

refactor: rework hook registration #468

Merged
merged 8 commits into from
Jun 14, 2024
Merged

Conversation

ProdPreva1l
Copy link
Contributor

New way to register a hook:

public class MyFancyHook extends Hook {
    @PluginHook(id = "MyFancyHook", register = PluginHook.Register.DELAYED, platform = "bukkit")
    public MyFancyHook(@NotNull HuskTowns plugin) {
        super(plugin);
    }
}

Copy link
Owner

@WiIIiam278 WiIIiam278 left a comment

Choose a reason for hiding this comment

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

top notch work, great new system! A few minor nits

@WiIIiam278
Copy link
Owner

Looks like this causes compile to fail during testing? Reflection might be breaking something here.

@ProdPreva1l
Copy link
Contributor Author

i think it has to with worldguardhook, that was the issue last time

@WiIIiam278
Copy link
Owner

it might be the Mockito trying to load the WG hook and going nuts for some reason.
is it crashing trying to load WG classes?

@ProdPreva1l
Copy link
Contributor Author

it might be the Mockito trying to load the WG hook and going nuts for some reason.

is it crashing trying to load WG classes?

i havent the slightest clue as to whats going wrong, maybe some bad accessors?

@WiIIiam278
Copy link
Owner

I did find that you're registering each hook twice

@ProdPreva1l
Copy link
Contributor Author

I did find that you're registering each hook twice

WHUUTT, lemme check this

@ProdPreva1l
Copy link
Contributor Author

Bump

@ProdPreva1l
Copy link
Contributor Author

Is there reviews that i cant see or am i tweaking bc i see "1 change requested" but no actual comments

@WiIIiam278 WiIIiam278 merged commit 1833cdd into WiIIiam278:master Jun 14, 2024
1 check passed
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.

3 participants