FastAsyncWorldEdit icon indicating copy to clipboard operation
FastAsyncWorldEdit copied to clipboard

Update plugin.yml

Open ExtremeFiretop opened this issue 1 year ago • 6 comments

Overview

Update softdepend list to include WorldGuard to stop the Loaded class error message: [ Vault, WorldGuard ]

Submitter Checklist

  • [x] Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • [x] Ensure that the pull request title represents the desired changelog entry.
  • [x] New public fields and methods are annotated with @since TODO.
  • [x] I read and followed the contribution guidelines.

ExtremeFiretop avatar Aug 03 '22 17:08 ExtremeFiretop

To trigger a test build on Jenkins, this PR requires approval by a member of the @IntellectualSites/admins team. Comment with one of the commands below:

Command/Comment Description
ok to test Accept this PR for testing
test this please Trigger a one time test
retest this please Start a new build if the last one wasn't successful

intellectualsites-bot avatar Aug 03 '22 17:08 intellectualsites-bot

test this please

ExtremeFiretop avatar Aug 03 '22 17:08 ExtremeFiretop

How did you test this out to make sure it doesn't affect dependencies negatively in circular and arbitrary environments? The bukkit plugin loader system isn't the greatest handling those cases.

NotMyFault avatar Aug 03 '22 18:08 NotMyFault

How did you test this out to make sure it doesn't affect dependencies negatively in circular and arbitrary environments? The bukkit plugin loader system isn't the greatest handling those cases.

All I did was modify the plugin.yml in FAWE and load it in paper on a Windows machine to confirm the expected functionality of both plugins (FAWE and WorldGuard) and that the Class Loaded Error message didn't appear. I didn't do anymore testing than that. Please let me know what tests you'd like performed and I can confirm the results.

ExtremeFiretop avatar Aug 03 '22 20:08 ExtremeFiretop

Looks fine to me, I have no server setup to test this properly at the moment.

NotMyFault avatar Aug 03 '22 21:08 NotMyFault

No rush of course; just a suggestion as I know people will poke about the warning if they see it.

ExtremeFiretop avatar Aug 04 '22 04:08 ExtremeFiretop

soft-depend shouldn't cause circular dependencies unless something goes very very wrong as it has no impact on load order. The trouble with load order where provides is used (i.e. in FAWE) has also been fixed iirc?

dordsor21 avatar Nov 25 '22 10:11 dordsor21

I'm surprised this hasn't been fixed yet, not sure what we are waiting for.

The softdepend is very well documented with Spigot and Bukkit, not sure what else would be expected of this report, FAWE devs just needs to do a bit of Googling.

I've been running this change on my own production servers since I originally reported this months ago without issues.

ExtremeFiretop avatar Nov 25 '22 22:11 ExtremeFiretop

  1. softdepend actually affects plugin loading order. If A softdepends on B, B is loaded before A (if B exists). The difference to depend is that A will load even if B is not present.
  2. The only reason this currently works is because the circular dependency check does not understand the provides section.
  3. There are a lot of other factors playing into this, like OS, file system, JDK version and other things we have no control over which could make this fail at any point. Just because it works right now and for you does not mean it is correct.
  4. The actual issue is a warning, not an error. It is meant to help with finding causes of a bug, but it isn't a bug itself.

I don't want to rely on the absolutely flawed mess of the current plugin loading system of spigot. Especially as the functionality only can get worse with this change.

SirYwell avatar Nov 27 '22 09:11 SirYwell

It should be noted that paper has plans to streamline the plugin loading process, see (https://github.com/PaperMC/Paper/pull/8108).

This will hopefully reduce any weird behavior but as a result plugin loading will no longer support circular plugin dependencies.

If I'm understanding this correctly, do you want to be able to add dependencies like this whilst not effecting the load order?

Owen1212055 avatar Nov 29 '22 13:11 Owen1212055

The problem is that we actually have a circular dependency. WorldGuard depends on WorldEdit (or FAWE providing WE), and FAWE optionally depends on WorldGuard (and also depends on it at compile time).

I hadn't the time to look into your Paper PR much, but I think the underlying problem is that plugins can randomly load classes from other plugins. In best case, this could be controlled, and no warnings for "unintended" class loading from other plugins are needed anymore.

SirYwell avatar Nov 29 '22 16:11 SirYwell

I see, so for FAWE it requires it during compile time therefore has to mark it as a dependency.

I defo see the use case when you need two plugins to be able to "integrate" with each other.

I think that eliminating this circular dependency behavior then might be problematic for plugins that do stuff like this, obviously in your case you rely on it as well.

Do you think then you'd benefit from another dependency-like configuration?

Current: hard depends,depends, and loads before (all effects loading order)

What if, for example, FAWE could use a new "Soft Utilizes" (idk a better name) option where FAWE would be able to say that it "utilizes" WorldGuard during runtime. This however wouldn't effect load order, so it would allow plugins to have a relationship like this. Is this something you'd benefit by? Or does FAWE rely on having worldguard on startup?

Would appreciate your feedback here. 😄

Owen1212055 avatar Nov 30 '22 01:11 Owen1212055

I see, so for FAWE it requires it during compile time therefore has to mark it as a dependency.

I defo see the use case when you need two plugins to be able to "integrate" with each other.

I think that eliminating this circular dependency behavior then might be problematic for plugins that do stuff like this, obviously in your case you rely on it as well.

Do you think then you'd benefit from another dependency-like configuration?

Current: hard depends,depends, and loads before (all effects loading order)

What if, for example, FAWE could use a new "Soft Utilizes" (idk a better name) option where FAWE would be able to say that it "utilizes" WorldGuard during runtime. This however wouldn't effect load order, so it would allow plugins to have a relationship like this. Is this something you'd benefit by? Or does FAWE rely on having worldguard on startup?

Would appreciate your feedback here. 😄

Yes this is beneficial.

ExtremeFiretop avatar Mar 05 '23 13:03 ExtremeFiretop