Skip to content

Conversation

Allmoz
Copy link
Contributor

@Allmoz Allmoz commented Jul 11, 2025

This is a second approach to solve #5420 (first one is #8774). This PR adds a Ghost Wrapper of ServelLevel to load the blocks and their entity data allowing for the full use of their .onRemove method when the block is deleted "virtually" dropping the block and it's content. The class is only instanced the first time is needed on the disassembly of a contraption. Also added the cases were is the target location isOutsideBuildHeight and when updateShape changes the state to AIR (solve #6988)

@Allmoz Allmoz force-pushed the contraption_container_reloaded branch 4 times, most recently from 34af35a to 8064502 Compare July 11, 2025 09:43
@VoidLeech VoidLeech added the pr type: fix PR fixes a bug label Jul 11, 2025
Copy link
Collaborator

@VoidLeech VoidLeech left a comment

Choose a reason for hiding this comment

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

This does look like a better solution to me (assuming it works), simply because there's no hardcoding every case, which will be more compatible with other mods and is less prone to breaking when a new feature is released.
Performance-wise this might perform slightly worse /w the level creation (but tbf that's only once per disassembly)

But kind reminder that the decision on what's better isn't mine to take (:

@@ -1144,6 +1144,8 @@ public void addBlocksToWorld(Level world, StructureTransform transform) {

translateMultiblockControllers(transform);

SingleBlockServerLevel popChamber = new SingleBlockServerLevel((ServerLevel) world);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is world actually always a serverlevel here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, i was assuming so since you can't add entities (drop items) if not. I can probably change it also to create the level only on the first fail

@Allmoz Allmoz force-pushed the contraption_container_reloaded branch from 8064502 to 88fc285 Compare July 11, 2025 22:18
@Allmoz
Copy link
Contributor Author

Allmoz commented Jul 11, 2025

thanks for the remainder, but i'm taking you down to the rabbit hole nonetheless :P (I really appreciate your help). Made the changes, now it will only create the level wrapper the first time a block "fails" to be placed and only if it's a server level. I did kept the sound event for both cases since the overridden destroy I did in the wrapped level is silent (no event is called there).

@Allmoz Allmoz force-pushed the contraption_container_reloaded branch from b99da53 to db63d97 Compare July 21, 2025 23:22
@Allmoz Allmoz force-pushed the contraption_container_reloaded branch from fda6965 to 3c612fc Compare September 11, 2025 01:34
@Allmoz Allmoz force-pushed the contraption_container_reloaded branch from 3c612fc to 579758b Compare September 11, 2025 01:42
@Allmoz
Copy link
Contributor Author

Allmoz commented Sep 11, 2025

conflicts were resolved :)

@Allmoz Allmoz changed the title Second Approach to Fix #5420 Second Approach to Fix #5420 & #6988 Sep 11, 2025
@Allmoz Allmoz marked this pull request as draft September 11, 2025 14:28
@Allmoz Allmoz marked this pull request as ready for review September 11, 2025 15:00
@Allmoz
Copy link
Contributor Author

Allmoz commented Sep 11, 2025

Changes added to fix also #6988

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr type: fix PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants