#487 - Add Plotsquared Flag #633
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Phoenix616
requested changes
Jun 9, 2025
Member
There was a problem hiding this comment.
Thank you for your PR!
A couple of things:
- I'm not fond of the thread sleeping to register the flag, see the comment on that for potential other approaches to solve that.
- Generally speaking I think it would be a bit cleaner if the flag's class was an inner-class of the PlotSquared class or at least named after PlotSquared. That way it is clear from the class-name or location which plugin it belongs to. (Seeing as both classes are rather small I would definitely prefer the inner-class approach if PlotSquared is able to handle such a setup)
- Please try to format the code like the rest of the project. It seems to be missing spaces between the keyword and the parenthesis. (It should look like
if (...)) Sorry for there not being an editerconfig or similar to auto-setup that, it's on my (too long) to-do :S
Comment on lines
+16
to
+34
| static { | ||
| // This is a workaround for the fact that PlotSquared's GlobalFlagContainer is not initialized at the time of plugin load. | ||
|
|
||
| ChestShop.runInAsyncThread(() -> { | ||
| com.plotsquared.core.plot.flag.GlobalFlagContainer flagContainer = com.plotsquared.core.plot.flag.GlobalFlagContainer.getInstance(); | ||
|
|
||
| while (flagContainer == null) { | ||
| try { | ||
| Thread.sleep(1000); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| flagContainer = com.plotsquared.core.plot.flag.GlobalFlagContainer.getInstance(); | ||
| if(flagContainer == null) continue; | ||
|
|
||
| flagContainer.addFlag(ALLOW_SHOP_FLAG_TRUE); | ||
| } | ||
| }); | ||
| } |
Member
There was a problem hiding this comment.
You should be able to just use the ChestShop#loadPlugin method which is called in the onEnable instead of the ChestShop#initializePlugin method from onLoad. Or is there a reason this needs to be done in onLoad? (If so you could also just call some kind of ChestshopAllowShopFlag#enable method to run this from ChestShop#loadPlugin)
I definitely don't like this hacky approach with sleeping a thread. Even just using a listener for the PluginEnableEvent would be cleaner but running the code from ChestShop#loadPlugin would still be my preferred approach.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds PlotSquared flag to ChestShop.
The Async task is neccesary because the GlobalFlagContainer gets constructed later in PlotSquared. We load dependencies in onLoad, but the container isn't available before the onEnable in PlotSquared.
The file is called
ChestshopAllowShopFlagbecause the name of the flag is constructed from the name of the class. There's no possibilty to override this.The name of the flag in game is
chestshop-allow-shop.PlotSquared can be compiled from source from https://github.com/IntellectualSites/PlotSquared
IssueHunt Summary
Referenced issues
This pull request has been submitted to: