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

add ALLOW_PLAYER_CHANGE_WORLD #4104

Closed

Conversation

fishshi
Copy link

@fishshi fishshi commented Sep 20, 2024

ALLOW_PLAYER_CHANGE_WORLD: Called when the player's dimension changes to check if the player is allowed to cross dimensions.

solve issue:
#3294 (comment)

@@ -64,6 +64,16 @@ public void onInitialize() {
LOGGER.info("Entity {} Killed: {}", entity, killed);
});

ServerEntityWorldChangeEvents.ALLOW_PLAYER_CHANGE_WORLD.register((player, origin, destination) -> {
if (player.getStackInHand(Hand.MAIN_HAND).getItem() == Items.END_ROD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
if (player.getStackInHand(Hand.MAIN_HAND).getItem() == Items.END_ROD) {
if (player.getMainHandStack().isOf(Items.END_ROD)) {

*/
public static final Event<AllowPlayerChange> ALLOW_PLAYER_CHANGE_WORLD = EventFactory.createArrayBacked(AllowPlayerChange.class, callbacks -> (player, origin, destination) -> {
for (AllowPlayerChange callback : callbacks) {
if (!callback.allowChangeWorld(player, origin, destination)){
Copy link
Contributor

Choose a reason for hiding this comment

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

Checkstyle will fail:

Suggested change
if (!callback.allowChangeWorld(player, origin, destination)){
if (!callback.allowChangeWorld(player, origin, destination)) {

Copy link
Author

Choose a reason for hiding this comment

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

thank you!

@fishshi fishshi force-pushed the feature/ALLOW_PLAYER_CHANGE_WORLD branch from 1fd397d to e36f266 Compare September 29, 2024 02:25
@@ -75,6 +88,19 @@ public interface AfterEntityChange {
void afterChangeWorld(Entity originalEntity, Entity newEntity, ServerWorld origin, ServerWorld destination);
}

@FunctionalInterface
public interface AllowPlayerChange {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be limited to just players, other entities are allowed to use portals.

I think its also worth looking at 1.21.2 as I know they changed a lot of this teleportation code.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this event is currently only used to restrict players when crossing dimensions; other entities are not affected. I believe there are very few scenarios where restricting entities from crossing dimensions would be necessary.

Copy link
Author

Choose a reason for hiding this comment

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

Additionally, I will read through 1.21.2 as soon as possible, and I may be able to provide feedback by tomorrow.

Copy link
Author

Choose a reason for hiding this comment

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

I have reviewed the relevant source code for 24w39a, and there are not many changes regarding the dimension-crossing code. Through testing, I can assure you that this event works correctly in the latest version as well.

@TelepathicGrunt
Copy link
Contributor

TelepathicGrunt commented Oct 18, 2024

Big concern with this PR. Say you block a teleport from a mod but that mod still sends the packets relating to teleporting. Won’t that cause issues and weird states in the game? For example, if you block the teleportTo in Bumblezone, my other packets will still be sent. https://github.com/TelepathicGrunt/Bumblezone/blob/2dc4c2034f1d9caf52cbe4c798754721b4895039/common/src/main/java/com/telepathicgrunt/the_bumblezone/entities/teleportation/BzWorldSavedData.java#L314

This is probably gonna cause major problems for the client and possibly other people as well

@modmuss50
Copy link
Member

Hi, thanks for the PR unfortunately I think doing this well requires quite a lot of work, teleportation is really complex. Some things that you will likely need to take into account:

  • None player entities
  • The context of how the entity is being teleported. E.g was it a command, portal, death, end credits or something another mod has initiated.
  • There should be 3 events, allow, on and canceled.
  • What state will the client and the entity being teleported be left in when the teleportation is blocked.

The first thing to do is to likely define a clear set of requirements/use cases, this likely requires looking at what existing mods do.

@fishshi
Copy link
Author

fishshi commented Oct 19, 2024

I'm really sorry, but I've decided to give up on this. Anyone else is welcome to continue trying.

@fishshi fishshi closed this Oct 19, 2024
@fishshi fishshi deleted the feature/ALLOW_PLAYER_CHANGE_WORLD branch October 24, 2024 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants