Skip to content

Fix bugs in collision detection when itemframe is outside the worldborder#9806

Closed
Hydrostic wants to merge 1 commit into
PaperMC:masterfrom
Hydrostic:patch-branch
Closed

Fix bugs in collision detection when itemframe is outside the worldborder#9806
Hydrostic wants to merge 1 commit into
PaperMC:masterfrom
Hydrostic:patch-branch

Conversation

@Hydrostic
Copy link
Copy Markdown

repair for #9805
in net.minecraft.world.entity.decoration.ItemFrame#survives

 @Override
    public boolean survives() {
        if (this.fixed) {
            return true;
        } else if (!this.level().noCollision((Entity) this)) {
            return false;
        } else {
            BlockState iblockdata = this.level().getBlockState(this.pos.relative(this.direction.getOpposite()));

            return !iblockdata.isSolid() && (!this.direction.getAxis().isHorizontal() || !DiodeBlock.isDiode(iblockdata)) ? false : this.level().getEntities((Entity) this, this.getBoundingBox(), ItemFrame.HANGING_ENTITY).isEmpty();
        }
    }

the itemframe will detect whether itself collides with other items by calling noCollision, but in net.minecraft.world.level.Level#noCollision(Entity, AABB), which has been modified by paper, hasn't consider the situation that itemframe is outside the world border and directly call the func getCollisionsForBlocksOrWorldBorder, which return false when entity is outside the world border

// Paper start - Prevent armor stands from doing entity lookups
    @Override
    public boolean noCollision(@Nullable Entity entity, AABB box) {
        if (entity instanceof net.minecraft.world.entity.decoration.ArmorStand && !entity.level().paperConfig().entities.armorStands.doCollisionEntityLookups) return false;
        // Paper start - optimise collisions
        int flags = io.papermc.paper.util.CollisionUtil.COLLISION_FLAG_CHECK_ONLY;
        if (entity != null) {
            flags |= io.papermc.paper.util.CollisionUtil.COLLISION_FLAG_CHECK_BORDER;
        }
        if (io.papermc.paper.util.CollisionUtil.getCollisionsForBlocksOrWorldBorder(this, entity, box, null, null, flags, null)) {
            return false;
        }

        return !io.papermc.paper.util.CollisionUtil.getEntityHardCollisions(this, entity, box, null, flags, null);
        // Paper end - optimise collisions
    }
    // Paper end

so I change the patch to

if (!(entity instanceof net.minecraft.world.entity.decoration.ItemFrame) && io.papermc.paper.util.CollisionUtil.getCollisionsForBlocksOrWorldBorder(this, entity, box, null, null, flags, null)) {
+            return false;
+        }

to prevent item frame from killed

@Hydrostic Hydrostic requested a review from a team as a code owner October 7, 2023 15:30
@Hydrostic Hydrostic changed the title Fix bugs in collision detect when itemframe is outside the worldborder Fix bugs in collision detection when itemframe is outside the worldborder Oct 7, 2023
@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Oct 7, 2023

I wonder if we can somehow pass if the entity noCollision should use the COLLISION_FLAG_CHECK_BORDER flag or not.
The instanceOf, while presumably fine, seems kind of meh there given we know the exact spot this check would have to trigger for so we could attempt to shortcut here a bit.

My brain is absolutely friend right now tho, so I sadly cannot look deeper into that.

@lynxplay lynxplay linked an issue Oct 7, 2023 that may be closed by this pull request
@Lulu13022002
Copy link
Copy Markdown
Contributor

Lulu13022002 commented Oct 7, 2023

Closing this pr since this is a hack not a fix, the proper way is to do what vanilla do instead of what the new system do. There's more issue with the world border collision. The item frame do indeed drop when near the border (<2*|boxSize|) but not when far outside of the border and that's the issue. Essentially in vanilla an entity collide with a border without collide epsilon and when the entity is near the border and inside (with a margin) but not far outside then the two voxel shape intersection are checked.

@Hydrostic
Copy link
Copy Markdown
Author

Closing this pr since this is a hack not a fix, the proper way is to do what vanilla do instead of what the new system do. There's more issue with the world border collision. The item frame do indeed drop when near the border (<2*|boxSize|) but not when far outside of the border and that's the issue. Essentially in vanilla an entity collide with a border without collide epsilon and when the entity is near the border and inside (with a margin) but not far outside then the two voxel shape intersection are checked.

If the border is set to expand automatically e.g. /worldborder add 10000 10,then will the moving border cause item frame to disappear?

@Lulu13022002
Copy link
Copy Markdown
Contributor

Yes, this might happen if the timing is right. For example if the border is slow enough that the item frame check its state (that happens each 100 ticks by default) and the condition are met.

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.

The Item frame outside the worldborder drops abnormally

3 participants