-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add safe-teleport detection to spawn points. #7530
Conversation
It would appear that this would prevent towns from placing any not-fully-solid blocks where players spawn, such as signs, buttons, pressure plates, and similar. |
This new behaviour should also only be enabled via a new config option that defaults to a false setting. |
Towny/src/main/java/com/palmergames/bukkit/towny/TownySettings.java
Outdated
Show resolved
Hide resolved
- ConfigNodes added empty string to comment - Uneeded diffs - Rename getSafetyTeleport to isSafeTeleportUsed
Do I have anything left to do for the PR to get pushed? Just to know if I forgot something |
Towny/src/main/java/com/palmergames/bukkit/towny/utils/SpawnUtil.java
Outdated
Show resolved
Hide resolved
Towny/src/main/java/com/palmergames/bukkit/towny/utils/SpawnUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a comment on the getSafeLocation(Location location, Player p)
method, and how it prefers to go downwards that should be addressed.
Towny/src/main/java/com/palmergames/bukkit/towny/utils/SpawnUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code backing this looks fine at this point. I'm still stuck on a point I made in an earlier comment:
I am a bit apprehensive about this method for a couple reasons:
I'm fairly sure that calling world.getBlockAt(Location) has to load the chunks to learn about the blocks. Part of using PaperLib to teleport players was making teleports less painful on servers. This new system will probably result in losing that performance gain. It is possible to load the WorldCoord that contains the spawn point though so this is something that can probably get solved for.
Do we know what sort of performance hit this feature adds yet?
Only one chunk will get loaded as this only looks in the Y axis, and it is the one the /t spawn is on, which would get loaded regarless, every check looks like it is performed in a completable future, the only sync operation that would happen is loading the chunk, and if I am correct all the calculation gets calculated async, iirc the expensive operation you talking about is getting blockStates, which we don't really need in this case, tell me if I am missing something |
with PR #7530. (First-Time Contributor!) - New detection system scans players' teleport destinations to make sure they will not suffocate or fall. - Closes #7463. - New Config Option: spawning.safe_teleport - Default: false - If enabled tries to find a safe location when teleporting to a town spawn/nation spawn/outpost.
Description:
Teleport a player near a safe location when teleporting to town spawn/outpost/nation spawn, for now it only fails the teleport
Relevant Towny Issue ticket:
#7463
By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.