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

Bring back the ITeleporter interface #6017

Closed

Conversation

rdvdev2
Copy link

@rdvdev2 rdvdev2 commented Aug 5, 2019

The implementation is very similar to the implementation on 1.12.x. The only change is that the placeEntity method has two new parameters: the ServerWorld objects of the origin and destination dimensions. This makes it easier to build exit portals and break entry portals if the modder needs to do so.

All the code is backwards-compatible:

  • Mods that were extending Teleporter will still work as expected (Teleporter instances are treated as vanilla ITeleporters)
  • Mods that copied the changeDimension method to modify it will still work as expected (Their code stays unchanged)

In both cases migrating to the ITeleporter approach should be easy and straightforward.

If I any change is required let me know it and I will solve it as soon as possible.

Resolves #5990

@tterrag1098 tterrag1098 added 1.14 Feature This request implements a new feature. labels Aug 5, 2019
@rdvdev2
Copy link
Author

rdvdev2 commented Aug 5, 2019

I somewhat figured out what the exc file was about. Let me know whether I did it correctly.

@SilverDavidMC
Copy link
Contributor

Is there any word or eta for if this will be merged? Seems pretty useful, especially for dimension mods like the Aether and Twilight Forest which relied on this.

d1 = MathHelper.func_151237_a(d1, d4, d6);
Vec3d vec3d1 = this.func_181014_aG();
blockpos = new BlockPos(d0, this.field_70163_u, d1);
- BlockPattern.PortalInfo blockpattern$portalinfo = serverworld1.func_85176_s().func_222272_a(blockpos, vec3d, this.func_181012_aH(), vec3d1.field_72450_a, vec3d1.field_72448_b, this instanceof PlayerEntity);
Copy link
Member

Choose a reason for hiding this comment

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

Your patches have vanilla code indented this bloats the patch size.

+ if (teleporter.isVanilla()) {
+ entity.func_174828_a(blockpos, entity.field_70177_z + f, entity.field_70125_A);
+ entity.func_213317_d(vec3d);
+ }
Copy link
Member

Choose a reason for hiding this comment

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

It may be better to wrap the entire section from Vec3d vec3d = this.getMotion(); to here, in a callable supplied to placeEntity.
THere isn't much vanilla code that modders would need to care about. So it isnt hard to reproduce.

+ vec3d = blockpattern$portalinfo.field_222506_b;
+ f = (float) blockpattern$portalinfo.field_222507_c;
+ } else {
+ teleporter.placeEntity(this, serverworld, serverworld1, this.field_70177_z);
Copy link
Member

Choose a reason for hiding this comment

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

placeEntity has a boolean value but isn't used here? Is there not a use case for preventing things from being able to teleport outside of players?

Copy link
Author

Choose a reason for hiding this comment

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

In vanilla, this boolean value is used to decide whether a new nether portal has to be built. I think using this same return value to prevent teleports or generate a portal depending on whether it's a vanilla teleporter wouldn't be very useful. Teleporting can already be prevented by canceling the EntityTravelToDimensionEvent so I think it would also be a little bit redundant.

+ } else if (!teleporter.placeEntity(this, serverworld, serverworld1, f2) && teleporter.isVanilla()) {
+ ((net.minecraft.world.Teleporter) teleporter).func_85188_a(this);
+ teleporter.placeEntity(this, serverworld, serverworld1, f2);
}
Copy link
Member

Choose a reason for hiding this comment

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

This has the vanilla implementation call logic inconsistently in the Entity vs Player patch.
Perhaps add a 'createNewEntity' argument. Or rely on the teleporter to do InstanceOf checks?

Copy link
Author

Choose a reason for hiding this comment

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

It isn't consistent? It calls placeEntity always and then the if body is called only if placeEntity returned false (vanilla behaviour) and the teleporter is a vanilla teleporter. I added the vanilla check to the if statement to make the patch smaller. The vanilla call logic is the same as it was before adding the ITeleporter interface.

import net.minecraft.world.server.ServerWorld;
import net.minecraft.world.server.TicketType;
+import net.minecraftforge.common.util.ITeleporter;
import org.apache.logging.log4j.LogManager;
Copy link
Member

Choose a reason for hiding this comment

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

Import line in patch is no go.

@@ -31,6 +31,8 @@ net/minecraft/client/resources/ClientResourcePackInfo.<init>(Ljava/lang/String;Z
net/minecraft/client/gui/ScreenManager.getScreenFactory(Lnet/minecraft/inventory/container/ContainerType;Lnet/minecraft/client/Minecraft;ILnet/minecraft/util/text/ITextComponent;)Ljava/util/Optional;=|p_216909_0_,p_216909_1_,p_216909_2_,p_216909_3_
net/minecraft/entity/EntityClassification.create(Ljava/lang/String;Ljava/lang/String;IZZ)Lnet/minecraft/entity/EntityClassification;=|name,p_i50381_3_,p_i50381_4_,p_i50381_5_,p_i50381_6_

net/minecraft/entity/Entity.changeDimension(Lnet/minecraft/world/dimension/DimensionType;Lnet.minecraftforge.common.util.ITeleporter;)Lnet/minecraft/entity/Entity;=|p_212321_1_,teleporter
Copy link
Member

Choose a reason for hiding this comment

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

This is horribly broken, and doesn't even address ServerPlayerEntity.
You should NEVER edit your patch files directly.
Please fix.

Copy link
Author

Choose a reason for hiding this comment

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

I'm working on solving the other issues but I really struggle with this. When I added the line to the exc file and regenerated patches nothing changed so I don't know if I'm missing some gradle task or whatever I'm supposed to do when adding files to the exc file. I really need some help here.

Copy link
Member

Choose a reason for hiding this comment

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

If your lines have no effect on the patch files, then your line is wrong.
It has to be explicit, it doesn't deal with inheritance.
You can get the signature/basically the full line from the bot on IRC/Discord {The AT Line} and change it to EXC format.

@tterrag1098
Copy link
Contributor

Any updates on this?

@tterrag1098 tterrag1098 added the Needs Update This request requires an update from the author to confirm whether the request is relevant. label Oct 11, 2019
@stale
Copy link

stale bot commented Dec 10, 2019

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

@stale stale bot added the Stale This request hasn't had an update from the author in a long time. label Dec 10, 2019
@FoxSamu
Copy link

FoxSamu commented Dec 10, 2019

Why so much silence? I think this is an important feature for me and everyone else who makes a dimension mod...

@stale stale bot removed the Stale This request hasn't had an update from the author in a long time. label Dec 10, 2019
@rdvdev2
Copy link
Author

rdvdev2 commented Dec 10, 2019

It is important even for me, that's why I started working on it, but I lack the time needed to work on this. If I can retake this I will, but until then, I invite anybody to fork my repo and finish the work.

@ichttt
Copy link
Member

ichttt commented Feb 4, 2020

This has been portet to 1.15 (#6404). If you want to backport it to 1.14, please use the 1.15 PR as a base instead of this one.

@ichttt ichttt closed this Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.14 Feature This request implements a new feature. Needs Update This request requires an update from the author to confirm whether the request is relevant.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.14.4] missing feature: player.changeDimension with teleporter argument
6 participants