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

Some additional solid checks #713

Merged
merged 1 commit into from
Dec 8, 2024
Merged

Conversation

Raycoms
Copy link
Contributor

@Raycoms Raycoms commented Dec 8, 2024

Closes #712
Closes #

Changes proposed in this pull request

  • Check if not replaceable/no collision for solidness too

Testing

  • Yes I tested this before submitting it.
  • I also did a multiplayer test.

Review please

@MotionlessTrain
Copy link
Contributor

I assume this PR will close the issue, rather than itself? 😛

@Raycoms
Copy link
Contributor Author

Raycoms commented Dec 8, 2024

I assume this PR will close the issue, rather than itself? 😛

Oui, I thought the issue was on minecolonies repo =D

@MotionlessTrain
Copy link
Contributor

I mean, the link in the “closes” section mentions this PR, rather than the issue

@Raycoms
Copy link
Contributor Author

Raycoms commented Dec 8, 2024

I mean, the link in the “closes” section mentions this PR, rather than the issue

oh, one number off xD

Copy link
Member

@Nightenom Nightenom left a comment

Choose a reason for hiding this comment

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

canBeReplaced makes somewhat sense to me
not sure if hasCollision is good idea

@@ -25,3 +25,5 @@ public net.minecraft.client.KeyMapping clickCount
# itemHandler
public net.minecraft.world.entity.decoration.GlowItemFrame getFrameItemStack()Lnet/minecraft/world/item/ItemStack;
public net.minecraft.world.entity.decoration.ItemFrame getFrameItemStack()Lnet/minecraft/world/item/ItemStack;

public net.minecraft.world.level.block.state.BlockBehaviour hasCollision # hasCollision
Copy link
Member

Choose a reason for hiding this comment

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

do we need this? i mean if it isnt avaiable through sth else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's the most efficient way to access it nicely.

@Raycoms
Copy link
Contributor Author

Raycoms commented Dec 8, 2024

canBeReplaced makes somewhat sense to me not sure if hasCollision is good idea

Blocks that have no collision aren't solid for our intents and purposes. (e.g. players/entities) can't stand on them.

Copy link
Member

@Nightenom Nightenom left a comment

Choose a reason for hiding this comment

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

This aint for "players/entities can stand of them" and shouldnt be used as such, if we're using it for this then we should have more definitions of solid
This solid is for building, ie.: if i place it at 0,0,0 will it under all conditions stay at 0,0,0 unchanged? (unchanged wrt. blockstate and blockentity serialization data)

@Raycoms
Copy link
Contributor Author

Raycoms commented Dec 8, 2024

This aint for "players/entities can stand of them" and shouldnt be used as such, if we're using it for this then we should have more definitions of solid This solid is for building, ie.: if i place it at 0,0,0 will it under all conditions stay at 0,0,0 unchanged? (unchanged wrt. blockstate and blockentity serialization data)

It's not a "floating block placeholder" it's a solid placeholder. A block that has no collision block is not solid.

@Nightenom
Copy link
Member

This changes I've done was for purpose of building being less error-prone, imho solid placeholders should really be tightened to sth like tag (and not to this rather weak check for building iterator)

@Raycoms Raycoms merged commit 6abb52d into version/1.21 Dec 8, 2024
5 checks passed
@Raycoms Raycoms deleted the additional-solid-checks branch December 8, 2024 21:05
Raycoms added a commit that referenced this pull request Dec 8, 2024
Check if not replaceable/no collision for solidness too
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