Multiverse-Core

Multiverse-Core

6M Downloads

PlayerPortalEvent firing even if allow-nether is false

seema84 opened this issue · 37 comments

commented
  • Server version: This server is running Paper version git-Paper-734 (MC: 1.16.5) (Implementing API version 1.16.5-R0.1-SNAPSHOT)

  • MV Core Version https://pastebin.com/NQ6w5x2Q

Description
mv core broke the "allow-nether=false" option in server.properties

Steps to reproduce

  1. set allow-nether=false in server.properties
  2. install mv core (only core not portals)
  3. nether portals work again
commented

Is there already news? I do not want to be a nuisance but the problem is already quite critical.

commented

Nope, I can't conclude a reproducible step after 2h of testing last week, it just works and stops working intermittently. Current there is 2 possible workaround:

  1. Use the older paper version that works
  2. Just disable portal forming for mv worlds with /mv modify set portalform none See https://github.com/Multiverse/Multiverse-Core/wiki/World-properties#portalform
commented

what may be important to reproduce, there must already be a nether world
if no world was created before, then the problem does not appear

commented

The thing is, from my testing, even with other worlds created with mv, some conditions actually let allow-nether to work but not always, so not that simple to conclude. Unfortunately had do other work, so we will see when I can work on this issue again. Noting that this isn’t the only issue I need to solve, hope you understand 😊

commented

if i disable portal forming, then players also cannot build netherportal farms
staying on an old version forever is not a good option either

if you need more detailed information, i will be happy to send it to you

commented

yes of course I understand that
it's just not a small thing but quite a big problem, that's why i ask so often

here again the steps to reproduce:

  1. install a fresh paper server and start this
  2. go into the nether and let the nether world create
  3. stop the server and set allow-nether=false in server.properties
  4. add the plugin mv core
  5. start the server

and then the netherportals work although it should be deactivated

commented

Ok I tested again, looks like allow-nether=false had a behaviour change. Instead of disabling then portal events, it just doesn't load the nether world on startup. But if you install mv already, mv will load it for you and basically, nether will be allowed since portal events are fired now (by right is shouldn't and this is NOT an mv bug).

So there are 2 ways to go about this current:

  1. Merge the linked PR to restore the previous behaviour. However, this will only be done if change in server software behaviour is deemed not a bug or a "wontfix" status.
  2. Use mv-netherportal, it already has the ability to prevent portal teleport by linking the world to itself. See https://github.com/Multiverse/Multiverse-Core/wiki/Basics-(NetherPortals)#customizing-linking

I think 2nd option is the way to go, as 1st one does have implications that can potentially affect other plugins installed on the server.

commented

the PR was not from me and a second portal plugin i dont need (is use https://www.spigotmc.org/resources/advanced-portals.14356/)
so mv core will not fix the problem?

commented

It's not an mv-core issue, allow-nether=false is by the server software and the server software behaviour changed. mv-core neither created nor cause the issue. This needs to raise the issue to the server software devs first before mv needs to do anything.

Also, advanced-portals has no relation to my suggestion on using mv-netherportal, not sure what you mean there.

commented

this is not correct, without mv core allow-nether=false works without problems

commented

I explain what I had to, I will make an issue report to paper and link it here soon.

commented

I explain what I had to, I will make an issue report to paper and link it here soon.

thanks @benwoo1110

commented

ben's explanation makes perfect sense! Nice investigation @benwoo1110 lol. I understand what the root issue is now, and I will try to fix it in Spigot. Paper says they will not fix it, but will accept a fix if it comes from Spigot.

commented

@nicegamer7 did you manage to fix it with spigot?

commented

I would also be interested, the bug is very annoying.

commented

I have opened a issue on spigot. Maybe someone can help to describe the problem in more detail.

https://hub.spigotmc.org/jira/browse/SPIGOT-6750?jql=text%20~%20%22allow-nether%3Dfalse%22

commented

Just an update (or rather, lack thereof). I opened a PR on the same day I posted my last comment here, but md_5 didn't seem to understand what the issue was, so it stalled and never got merged.

commented

this is almost always the case with md_5

commented

So if allow-nether=false, nether portals still teleport to the nether?

commented

So if allow-nether=false, nether portals still teleport to the nether?

yes

commented

Same with allow-end but it seems to only appear if world_the_end exists

commented

I tried a lot of different paper builds and it seems broken by paper-1.16.5-664.

It is a problem from Multiverse because without, players don't teleported.

commented

Interesting... that's the version that includes a fix I submitted that allows you to reload the main nether and end worlds. I don't understand how that would cause this.

Nevertheless, I will look into it. Thanks for taking the time to see which build this behaviour changed.

commented

Weird lol, the portal event must be triggered before even mv can do anything about it. MV can’t trigger it by itself. Also noting that it only happened after a certain paper built points to an issue with the server software.

Usually not cause by mv, but mv just got effected by certain bugs/changes from the server software.

commented

I agree with ben.

commented

Yeah , i agree with you but in that case, there is something that i don't understand :
You are checking sometimes that event.getFrom() is not null but it is marked as @NotNull in the PlayerMoveEvent class so it shouldn't ever be null.

image

In my opinion, it's exactly the same thing as this issue. The server shouldn't throw / trigger something but you are checking it anyway.

commented

Likely it was done years ago when there wasn't a NotNull annotation in Bukkit api. But it won't effect the issue you raised up. In 1.16, when allow-nether is false, PlayerPortalEvent is not be called at all, at least until that paper build. So you should focus the attention to what changed in paper that will cause such an issue. (Maybe report to paper)

Only if its deemed as a permanent behaviour change that paper wish to not fix, then yes, we will have to think of how to mitigate this on our end.

Edit: Btw when I say it doesn't trigger, means the method in playerPortalCheck method in MV is complete not ran. So the null checking is not relevant in this case.

commented

Is there a solution to the problem yet? After all, this is a high-priority problem.

commented

No, since it looks like an issue by the server software.

commented

In my opinion, @nicegamer7 is the most indicated to patch this bug.
If you want to resolve the issue now, you can always build the plugin from my branch : https://github.com/PapiCapi/Multiverse-Core

commented

Maybe since spigot 1.17 is out, can you try on a fresh install of that to see if you can reproduce the issue? I am still not convince its something cause by mv.

commented

Maybe since spigot 1.17 is out, can you try on a fresh install of that to see if you can reproduce the issue? I am still not convince its something cause by mv.

Yes i can do that.
But why is the problem only when Multiverse is also running as a plugin on the server. Without MV the problem is not present.

commented

Has the problem been reported to Spigot/Paper yet?

commented

No, bcu no one has the time to look into it yet except the report that it only stop working after a specific paper build. Which basically just means something has changed in paper that caused the issue.

commented

Should I create a issue with Paper and ask what was changed?

commented

No clue, that we need ppl to test. As it stands, the evidence provided points to an issue with paper. I won't have time to open a test server anything this week.

commented

The problem is also present in the first 1.17 (Paper-1.17-R0.1-EXPERIMENTAL) version of Paper.