Fix Legacy Portal Detection

[nicegamer7]

This PR fixes legacy portal detection. Although, it seems it wasn't used anywhere, it's important that it works properly for anyone who wants to use the API. I also made it so that the Player Move event handler only handles legacy portals, but this exposed a bug. It seems the player portal event handler doesn't work properly for normal portals. I tested this on 1.16.3.

[nicegamer7]

The above mentioned issue is fixed, but that exposed another bug which is fixed by #600. So, this is on hold until that fix is merged.

[nicegamer7]

Here's a test build of this PR.

Multiverse-Portals-4.2.2-SNAPSHOT.jar.zip

[benwoo1110]

Actually, what is a "legacy" portal? Like portals made on an older version of Minecraft?

[nicegamer7]

Dumptruckman and I talked about this. "Legacy" is a bad term, and we're gonna rename it in 5.0.0.

"Legacy" is any portal that isn't filled with the nether portal block. So, for example, a portal that is made of water is considered "Legacy".

[benwoo1110]

I tested a bit and this is definitely more robust in terms of detecting nether_portal type.

However, there is one change, portals with nether_portal fill will only teleport after the "wavy animation" instead of immediately. I also notice this animation only happen in survival, not creative (probably a client or server side thing)

I suggest having a boolean config option netherportalanimation. If this is false, we do the teleport at EntityPortalEnterEvent instead so the animation doesn't happen.

Now onto the bug relating to portalsdefaulttonether, where even if its false, the player is still teleported the default nether destination. This is due to pm.getPortal(event.getPlayer(), playerPortalLoc) returning only portals with permissions to access. So if no permission, it will return null and PlayerPortalEvent will never be cancelled.

[nicegamer7]

About the animation, that's the vanilla behaviour (animation in survival, instant in creative). I don't think I've heard of anyone who wanted to change it, but I suppose an option wouldn't hurt.

[benwoo1110]

Yea, but I think it's good to have an option to not have it even in survival mode - since that was the behaviour before this PR

[nicegamer7]

I suppose so, yeah.

[nicegamer7]

I'll work on the config option later today, unless you want to (I might be a bit slow).

[benwoo1110]

Very busy with school, will review it when you add it 😄

[nicegamer7]

I completely forgot this was MV Portals (I was thinking MV NetherPortals the whole time 🤦‍♂️). Now I understand why the config option would be nice lol.

[benwoo1110]

HAHA yea I mix them up at times too

[benwoo1110]

Looks good, config option will be done in a separate pr.

[Phoenix616]

Looks good, config option will be done in a separate pr.

Is this still planed? I actually have some players being confused about portals no longer teleporting directly if it is using nether portal blocks. 👀

[nicegamer7]

Yes, it's still planned. Albeit I did forget about it. Thanks for the reminder.