Skip to content

Implement again save for position of last lava contact#11991

Closed
Doc94 wants to merge 2 commits into
PaperMC:mainfrom
Doc94:fix/handle-lava-damage-for-block
Closed

Implement again save for position of last lava contact#11991
Doc94 wants to merge 2 commits into
PaperMC:mainfrom
Doc94:fix/handle-lava-damage-for-block

Conversation

@Doc94
Copy link
Copy Markdown
Member

@Doc94 Doc94 commented Jan 19, 2025

This PR resolve #11984 by adding again set the position of last lava contact, also add the Nullable annotation to that field.

@Doc94 Doc94 requested a review from a team as a code owner January 19, 2025 14:32
@Doc94
Copy link
Copy Markdown
Member Author

Doc94 commented Jan 19, 2025

now.. im not good with the large feature touchs... im not sure if this is the better way for handle the add again that CB patch.. or if is better fix that in the source file and the moonrise patch just need avoid deleting that...

@electronicboy
Copy link
Copy Markdown
Member

The issue here is that we don't really want to be implementing this stuff into moonrise directly, as it will just be removed on the next moonrise update

@Doc94
Copy link
Copy Markdown
Member Author

Doc94 commented Jan 19, 2025

The issue here is that we don't really want to be implementing this stuff into moonrise directly, as it will just be removed on the next moonrise update

then the fix its just modify the file and handle the merge conflict or exists a better way?

@electronicboy
Copy link
Copy Markdown
Member

cept there will be no merge conflict due to how this stuff is applied, we need MR to add a hook or something so that the changes don't get wiped out on every update

@Doc94 Doc94 force-pushed the fix/handle-lava-damage-for-block branch from db992d8 to 5f49c9b Compare January 19, 2025 15:34
@Doc94
Copy link
Copy Markdown
Member Author

Doc94 commented Jan 19, 2025

Ok i remake this... just copy the part of updateFluidHeightAndDoFluidPushing where get the last BlockPos for the fluid... and use that for update the BlockPos of lastLavaContact this require keep update that new method but avoid mess with the method touched by moonrise.

image

@lynxplay lynxplay added the type: bug Something doesn't work as it was intended to. label Jan 26, 2025
Copy link
Copy Markdown
Member

@Owen1212055 Owen1212055 left a comment

Choose a reason for hiding this comment

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

In general, I do not think this is the correct solution.

We will need to work with moonrise to add a hook into that method, which then through PaperHooks we could add the field setting there. I have notified them internally.

@github-project-automation github-project-automation Bot moved this from Awaiting review to Changes required in Paper PR Queue Mar 8, 2025
@lynxplay
Copy link
Copy Markdown
Contributor

Fixed in 1.21.5 via mojangs InsideBlockEffectApplier

@lynxplay lynxplay closed this Apr 14, 2025
@github-project-automation github-project-automation Bot moved this from Changes required to Closed in Paper PR Queue Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something doesn't work as it was intended to.

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

EntityDamageByBlockEvent / EntityCombustByBlockEvent returning null block for lava

4 participants