Implement Entity#setPosition and clean up some related logic #3484
Conversation
dualspiral
added
type: bug
src/mixins/java/org/spongepowered/common/mixin/core/world/entity/EntityMixin.java
Outdated
Show resolved
Hide resolved
src/mixins/java/org/spongepowered/common/mixin/core/world/entity/EntityMixin.java
Outdated
Show resolved
Hide resolved
|
|
||
| final ChunkPos chunkPos = new ChunkPos((int) this.shadow$getX() >> 4, (int) this.shadow$getZ() >> 4); | ||
| destinationWorld.getChunkSource().addRegionTicket(TicketType.POST_TELEPORT, chunkPos, 1, ((Entity) (Object) this).getId()); | ||
| return this.impl$teleportToWithTicket(destinationPosition.x(), destinationPosition.y(), destinationPosition.z(), false); |
There was a problem hiding this comment.
Here is a good discussion.
Does setting the location of a non player trigger a chunk ticket? Should we be doing that?
There was a problem hiding this comment.
If the chunk isn't loaded and the entity is moved there, where would the entity be saved to? Does the chunk need to be loaded to save the entity's location to it?
There was a problem hiding this comment.
That is a good question dual. In fact, it is likely premature to discuss this now since entities still exist in Chunk files in 1.16.5. In 1.17, they exist outside chunk files and it likely makes a lot of sense to reposition them and do a save without triggering a chunk load.
There was a problem hiding this comment.
That's what I thought, I knew a split happened in 1.17.x - so I suspect that the answer is we need the ticket in 1.16, revisit in 1.17...
* Also some related cleanup, specifically making bridges that shouldn't be bridges not bridges.
Superscedes #3442
With many thanks to @Lignium for starting this. The further changes attempts to unify some of the move event handling and reduce duplication. Some cleanup has occurred too, converting methods that didn't actually need to be bridge methods to
impl$methods.I want to test it once more tomorrow before I merge.