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

Delay economy initialization to server load #4216

Merged
merged 1 commit into from
Oct 29, 2023
Merged

Conversation

SirYwell
Copy link
Member

Overview

Description

Guice initializes Singletons eagerly, i.e. as soon as we initialize guice itself when enabling the plugin. However, economy implementations might load after PlotSquared (both an economy plugin and P2 will load after Vault, but their load order is not defined).

As a solution, we can wrap the EconHandler and only initialize it once the server is ready. This ensures that all plugins are loaded. We can still fall back to the NullEconHandler if no plugin provides economy by then.

Submitter Checklist

Preview Give feedback

@SirYwell SirYwell requested a review from a team as a code owner October 26, 2023 08:22
@github-actions github-actions bot added the Bugfix This PR fixes a bug label Oct 26, 2023
@@ -128,21 +133,64 @@ protected void configure() {
@Provides
@Singleton
@NonNull EconHandler provideEconHandler() {
if (!Settings.Enabled_Components.ECONOMY) {
if (!Settings.Enabled_Components.ECONOMY || !Bukkit.getPluginManager().isPluginEnabled("Vault")) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we definitely always require Vault?

Copy link
Member

Choose a reason for hiding this comment

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

Seems so

@NotMyFault NotMyFault merged commit 1c3776b into main Oct 29, 2023
7 checks passed
@NotMyFault NotMyFault deleted the fix/delay-econ-init branch October 29, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants