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

Nether portals #1597

Merged
merged 18 commits into from
Feb 3, 2021
Merged

Nether portals #1597

merged 18 commits into from
Feb 3, 2021

Conversation

tastybento
Copy link
Member

Closes #1595 when merged.

This adds two settings to WorldSettings (current set to true by default for testing), that will enable the use of default portals in game modes via a config.yml setting.

When a player goes through a nether portal in the overworld, they will be sent to the nether at the same coordinates as where they left. Normal vanilla rules apply as to where the portal is made but it's usually that location and will just overwrite any blocks that are currently in that location.

For the end portal - when players go through the end portal, the server will generate an obsidian platform for them. It will overwrite any blocks in that location.

I'm happy to include this option, but it has some known issues listed below.

Note that the current implementation in BentoBox was designed to avoid the endless obsidian exploit. Even if the obsidian blocks are valued at 0 by the level addon they are very good for creeper protection and can be given to others for portal creation or used to create massive portals for gold farming.

The current design does send players to an active portal if one exists in the nether. So that mechanic is the same as vanilla. The rest of the operations are essentially the same as vanilla except that the obsidian blocks are not made.

Known issues/exploits with this option

  • Players can harvest unlimited obsidian and bring it back to the overworld, so the Level addon should set Obsidian value to 0 or limit counting to about 10 blocks or so. It can be used for gold farming by making many portals.
  • Players are teleported by the server to the nearest live portal. Portals usually have to be 128 blocks apart to be considered different. This could result in players arriving on an adjacent island's portal instead of their own being generated.
  • By placing a portal right on the edge of an island's protection range, the resulting nether portal or end platform can extend outside of the island's range and be unbreakable.

Currently defaulted on for testing. Ultimately, the game mode config can
decide if the vanilla portal is used or not.
Note that the end platform is just a set of obsidian blocks.
@tastybento tastybento requested a review from BONNe December 6, 2020 18:38
@tastybento tastybento added the Type: Enhancement Improvement or modification which is usually a new feature. label Dec 6, 2020
@BONNe
Copy link
Member

BONNe commented Dec 6, 2020

Nice, that was fast...

But not working well with [spawn_here] signs in any dimension.
Portals are always created in their locations unless there exists a portal on the island.

It does not look right, that the first check location is the spawn point. I think it should be the location of the portal if this feature is enabled.

Is there a way how to deal with 128 portal check radius?

@tastybento
Copy link
Member Author

Nice, that was fast...

I had some free time. 😄

But not working well with [spawn_here] signs in any dimension.
Portals are always created in their locations unless there exists a portal on the island.

It does not look right, that the first check location is the spawn point. I think it should be the location of the portal if this feature is enabled.

Aha, yes. I'll look at that.

Is there a way how to deal with 128 portal check radius?
Not that I know of. Once you leave it up to the server it'll do what it does. This is why it might be good to use the spawn point and not the overworld location. That way it can be controlled.

@BONNe
Copy link
Member

BONNe commented Dec 6, 2020

Hmm, Spigot Java docs lists search radius and create radius for player portal event.
https://hub.spigotmc.org/javadocs/spigot/org/bukkit/event/player/PlayerPortalEvent.html

@tastybento
Copy link
Member Author

Hmm, Spigot Java docs lists search radius and create radius for player portal event.
https://hub.spigotmc.org/javadocs/spigot/org/bukkit/event/player/PlayerPortalEvent.html

That's useful! Hmm, in that case, the radius should be set if the location is close to the edge of the island protected zone, reducing to 1 if the portal is on the edge. Agree?

@BONNe
Copy link
Member

BONNe commented Dec 10, 2020

I was thinking about it, and I think yes.
However, the search distance and creation radius should be configurable. At least search radius.

But I agree, that if someone places a portal on the border with another island, it should do not search inside other player islands.

@tastybento
Copy link
Member Author

BONNe reports:

I found several bug with BentoBox Portal Teleportations:

If player uses End Exit portal, then he is not teleported back to his island.

SafeSpotTeleport does not except PortalBlocks, so players can easilly be teleported inside portal, and back to overworld (if that would be fixed).
SafeSpotTeleport does not except EnderCrystals, so players get on fire as soon as they are teleported into them... or died if they hit it.

@BONNe
Copy link
Member

BONNe commented Dec 19, 2020

It does not look like it was a bento box fault...
For some reason, the custom end portal do not fire teleport event and just move a player to some weird location...

I created a listener that should fix it and fire the event manually:
https://github.com/BONNePlayground/DragonFights/blob/develop/addon/src/main/java/lv/id/bonne/dragonfights/listeners/EndTeleportListener.java#L40

However, I notice, that you used a movement event in detecting when a player enters into portal if nether and the end is disabled into server settings.

I suggest using the same event - EntityPortalEnterEvent, as it would be fired only when a player enters into the portal, instead of as it is now with the current implementation, that is performing checks on each player movement.

@tastybento
Copy link
Member Author

I made the change you suggested @BONNe . I need to test it with a system with nether and end disabled to confirm.

@BONNe
Copy link
Member

BONNe commented Dec 19, 2020

The only thing that I currently do not like with this implementation, is that the spawn-point will be always chosen over the actual portal location.

I would prefer it to be toggleable, or switched in the opposite order. As with the end, it is just not usable, due to the fact, that in the end, the spawn point will create an obsidian platform ... and it will always happen in the middle of the island without an ability to change it at all.
The same will be always true for the nether, but there you can at least create your own portals and link them, but with the end.. .you cannot do that.

Also, I would prefer to split these even catchers in 2 parts -> overworld-nether and nether-overworld.... it would improve readibility.

Unfortunately, PlayerPortalEvent and EntityPortalEvent are not fraternal
classes so there's no way to apply code to both except via this
abstraction class.

Tests fail.
@tastybento
Copy link
Member Author

I rewrote the code to be more generic and remove duplication. I also enabled entity teleporting. Notes:

  1. The default setting in WorldSettings is false, so if you want to enable portal generation, then change that to true or override in a game mode addon.
  2. For some reason, some entities teleport fine, others seem to disappear. I only tested with cows and villagers and cows just disappeared after teleporting. Villagers did not. I don't know why.
  3. Tests need to be updated and written.

Please clone it and try it.

}
// Custom portals
e.setCancelled(true);
// If this is from the island nether or end, then go to the same vector, otherwise try island home location
Copy link
Member

Choose a reason for hiding this comment

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

The comment for the code below is telling different than the actual code. One of them is wrong.

@BONNe
Copy link
Member

BONNe commented Dec 21, 2020

The end portal platform generation is not working properly.
Currently, the platform location is not fixed to a portal location, but to the block in which the player enters. It means that it generates up to 12 different platforms.

It should not happen, and for the end, it should normalize portal location, f.e. always generate portal in the middle block of THE END enter portal.

It looks like the latest changes with code refactoring break the allow-nether=false hack. If nether is disabled, portals do not work.

Entities that enter the end portal, are teleported to the nether world :D It does not matter if it is an exit or enters portal.

If BentoBox end islands are disabled, end portal handling should use default behavior, with no custom location calculations.
Otherwise, BentoBox replaces the exit portal with the obsidian platform.

Update:
Also, I created a spigot bug report: https://hub.spigotmc.org/jira/browse/SPIGOT-6283. Apparently, the existing end does not trigger any event.

Update 2:
I think we should allow teleporting all entities, regardless of animals or items. Or it should be toggleable.

@tastybento tastybento self-assigned this Dec 21, 2020
@tastybento tastybento added the Status: In progress Working on the issue. label Dec 21, 2020
Identify the teleport cause manually because there is no method.
Teleports to the End happen but seem to be slightly different locations.
Some entities will disappear, others will stick around. I don't know
why.
@tastybento
Copy link
Member Author

I fixed some of the things you mentioned. I'll work on the others tomorrow.

@tastybento
Copy link
Member Author

Okay, here's the status:

The end portal platform generation is not working properly.
Currently, the platform location is not fixed to a portal location, but to the block in which the player enters. It means that it generates up to 12 different platforms.
It should not happen, and for the end, it should normalize portal location, f.e. always generate portal in the middle block of THE END enter portal.

This is fixed. To get the location and make it the same every time I had to search for the portal blocks and then locate the corner block. I arbitrarily chose one of the corners as the reference point for speed. The next challenge was that players could "punch" through the portal either by dropping through the portal or rising up fast before the event fired, so I had to search in the y axis as well sometimes, so that the search became more complex. Anyway. it works. The server generates the obsidian platform based on the location given and it's the same no matter where you drop in the end portal or how fast you go through it.

It looks like the latest changes with code refactoring break the allow-nether=false hack. If nether is disabled, portals do not work.

No, actually this is because by disabling the nether or end you are switching off the vanilla teleporting behavior, so it does not teleport you. The option to use the vanilla teleporting approach is only compatible if the nether and end are enabled at the server level. Essentially, this PR is stepping aside and letting the server do the work of teleporting, generating the portals, searching for space, etc. etc. and if the configs switch off the nether or end then that codes does not run.

Entities that enter the end portal, are teleported to the nether world :D It does not matter if it is an exit or enters portal.

This was because the EntityTeleportEvent made me think it only applied to nether portals because it doesn't have a getCause() method, which is available in the PlayerTeleportEvent. As a result I have to search around the entity's location for a portal block and deduce what type of portal it entered. I only search 1 block in every direction and stop as soon as it's found for speed. The location given in the event is not a portal block so the search is necessary.

If BentoBox end islands are disabled, end portal handling should use default behavior, with no custom location calculations.

If end islands are disabled and there is a common end world then the teleport should just go to the world spawn location. Where else should it go?

Otherwise, BentoBox replaces the exit portal with the obsidian platform.

The obsidian platform is made by the server if you use vanilla teleporting. Tell me more about this issue so I understand what should be done here.

I think we should allow teleporting all entities, regardless of animals or items. Or it should be toggleable.

Done. Anything can go through - however, weirdly, and I don't know why, some entities never pop out on the other side:

  • Skeletons
  • Pigs
  • Cows

Entities that do:

  • Dropped diamonds, torches
  • Villagers
  • Minecarts

I haven't tried everything, but I don't understand why they don't transfer. I'm only testing with one player, so maybe they get instantly removed...

@BONNe
Copy link
Member

BONNe commented Dec 23, 2020

About end portal:
If end islands are disabled, portal is generated at world spawn, so it is 0:0 location.
At that location exist end exit portal, as that is vanilla location.
We could generate portal at the same location as vanilla, i just need to check if it is always the same.

About entities:
For me cows was working. Will test with other entities today.

Thanks for the progress.

@BONNe
Copy link
Member

BONNe commented Dec 23, 2020

I can confirm, that the vanilla END spawn point is always at 100, 50, 0 unless someone changes them manually.

So it means that we can use that location if end islands are disabled. It is very much hard coded thing.

And about entities:
To the nether and back all entities work for me.
To the end from overworld, it does not work at all, entities disappear.
To the end from the nether, entities are spawned at the island center location, instead of the portal location and generated obsidian platform.

And from end to overworld, players and entities appear in the default world spawn point. But it is a bug in a spigot, that they do not trigger events.

Update:
I manage to get entities from overworld to the end, and they too spawn on the end island spawn point. (I had a return portal at that location, so they got teleported away instantly)

@tastybento
Copy link
Member Author

I can confirm, that the vanilla END spawn point is always at 100, 50, 0 unless someone changes them manually.
So it means that we can use that location if end islands are disabled. It is very much hard coded thing.

Right now, the teleportation code will teleport to the world spawn point. What that means is that the game modes should set the default spawn point for the the end to 100, 50, 0 if a new world is created. Like this for BSkyBlock:

// Make the end if it does not exist
        if (settings.isEndGenerate()) {
            boolean firstTime = getServer().getWorld(worldName + THE_END) == null;
            if (firstTime) {
                log("Creating BSkyBlock's End World...");
            }
            endWorld = settings.isEndIslands() ? getWorld(worldName, World.Environment.THE_END, chunkGenerator) : getWorld(worldName, World.Environment.THE_END, null);
            if (!settings.isEndIslands() && firstTime) {
                // Set the end world's default spawn point - this is where it always is
                endWorld.setSpawnLocation(100, 50, 0);
            }
        }

I'll look at the entities right now.

@BONNe
Copy link
Member

BONNe commented Dec 23, 2020

That should be noted in FAQ when you merge this into the main branch.
I assume that not all will know about it, and not all have it in existing worlds.

@tastybento
Copy link
Member Author

@BONNe I actually managed to set the default spawn point in BentoBox so it doesn't need to be in the addons. It checks if there's a default spawn point set and changes it to what you said. It results in the obsidian platform being below the end island, which I assume is where it is supposed to be.

@BONNe
Copy link
Member

BONNe commented Jan 20, 2021

I think this is implemented, however it could be awesome to support also #1632 as it is a similar usage.

https://github.com/BentoBoxWorld/BentoBox.git into nether_portals

Conflicts:
	src/test/java/world/bentobox/bentobox/listeners/PortalTeleportationListenerTest.java
@tastybento
Copy link
Member Author

I'm going to merge this so we can get some time using it before release.

@tastybento tastybento merged commit 4e7b788 into develop Feb 3, 2021
@tastybento tastybento removed the Status: In progress Working on the issue. label Feb 3, 2021
@Poslovitch Poslovitch added this to the 1.16.0 - Home sweet home... milestone Feb 7, 2021
@Poslovitch Poslovitch added the Status: Done This issue has been completed or answered. This pull request has been merged. label Feb 7, 2021
@Poslovitch Poslovitch deleted the nether_portals branch March 13, 2021 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Done This issue has been completed or answered. This pull request has been merged. Type: Enhancement Improvement or modification which is usually a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proper Nether / End Portals
3 participants