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

Hopper cache invalidation broken when inventory blocks are placed without block updates (WorldEdit) #417

Open
Fallen-Breath opened this issue Nov 17, 2022 · 12 comments
Labels
bug Something isn't working confirmed The issue has been confirmed by a project maintainer or through multiple user reports good issue report This is an example of a good issue report mod compatability This issue affects compatibility with other mods

Comments

@Fallen-Breath
Copy link

Fallen-Breath commented Nov 17, 2022

Instructions

If a inventory block with contents inside is placed without updates next to a hopper, the hopper will not pull / push items from / to the inventory block


Version Information

  • Minecraft 1.19.2
  • fabric-carpet
  • lithium-fabric-mc1.19.2-0.10.2

Reproduction Steps

  1. Use /carpet fillUpdates false command to enable a carpet rule that removes block updates when during /setblock or /clone commands
  2. Place hopper. Place a chest and put something inside the chest
  3. Use /clone command to clone the placed chest to the top of the hopper

Expected Behavior

Hopper starts pulling items from the chest

Actual Behavior

Hopper does nothing and refuse to pull items from the chest. Seems like the hopper is not notified

Other Information

With /carpet fillUpdates false, using /setblock command to set a chest with inventory also doesn't make the hopper work

If you use worldedit and do //perf neighbors off to disable block updates from worldedit operation, then worldedit operations with inventory blocks also doesn't notify the hopper. Reproduced with worldedit 7.2.12 in mc1.19.2

lithium 0.10.2 for mc1.18.2 has this issue too

reloading the chunk fixes the "broken" hopper

@Fallen-Breath Fallen-Breath added the bug Something isn't working label Nov 17, 2022
@2No2Name 2No2Name added confirmed The issue has been confirmed by a project maintainer or through multiple user reports good issue report This is an example of a good issue report labels Nov 17, 2022
@2No2Name
Copy link
Member

This issue is caused by lithium assuming that the placement of an inventory block will send block updates to sourrounding hoppers. The block updates are used to invalidate the cached information which says that the hopper has no inventory block to interact with.
This probably also affects removal of composter blocks.

@2No2Name
Copy link
Member

For a fix we probably have to check how carpet and world edit implement the update suppression. At some point we can probably get an update out of the block creation, however this might decrease the performance of large fills.

@2No2Name 2No2Name added the mod compatability This issue affects compatibility with other mods label Nov 17, 2022
@Zifiv
Copy link

Zifiv commented Nov 26, 2022

Hello @2No2Name,

Same issue on Farmer's Delight Fabric port, without Lithium, the cooking pot is working well, but if last version of Lithium is installed (I've not yet tested with older version), hoppers cannot pull or push items in the cooking pot until they are updated themself.

My first guess is that I'm doing something wrong in the code that break the interaction, but it work well without Lithium, and the code is the same than a furnace block.

If you need any information to understand what's wrong, I'm available.

This will maybe fixed by this action https://github.com/CaffeineMC/lithium-fabric/actions/runs/3504930287, so in next release, but I prefered notify that the bug.

@2No2Name
Copy link
Member

Ideas how to fix update suppressed placement of inventories near hoppers not updating the hopper's caching:

  1. Hook into some vanilla code that still runs (e.g. setblockstate), which won't be foolproof and might cause lag in other scenarios if not paying close attention to it
  2. Hook into other mods's code
  3. Provide an API to send updates to lithium's hoppers which the other mods can use

I think I will look into 1. as the other options are probably more work and less maintainable

@2No2Name
Copy link
Member

@Zifiv please move to #423 , this issue about something different I believe

@2No2Name
Copy link
Member

@Fallen-Breath I think the build at https://github.com/CaffeineMC/lithium-fabric/actions/runs/3554890604 might be able to fix the issue. If you test it, your feedback will be appreciated a lot.

@2No2Name
Copy link
Member

The way it is fixed may or may not work with world edit, since I only read the fabric-carpet code in detail so far

@Fallen-Breath
Copy link
Author

@Fallen-Breath I think the build at https://github.com/CaffeineMC/lithium-fabric/actions/runs/3554890604 might be able to fix the issue. If you test it, your feedback will be appreciated a lot.

The github action failed, might due to this client-side only import

import static net.minecraft.client.render.WorldRenderer.DIRECTIONS;

@2No2Name
Copy link
Member

Ugh, should have... tested it or waited for the build to finish.

@2No2Name
Copy link
Member

Again, not testing before commenting:
https://github.com/CaffeineMC/lithium-fabric/actions/runs/3561253591

@Fallen-Breath
Copy link
Author

Fallen-Breath commented Nov 28, 2022

Again, not testing before commenting: https://github.com/CaffeineMC/lithium-fabric/actions/runs/3561253591

Works for fabric carpet's /carpet fillUpdates false, but still doesn't work correctly with worldedit

For worldedit's implemetation, you can check these 2 classes. Basically worldedit directly sets block in chunk, and then performs side-effects by itself

https://github.com/EngineHub/WorldEdit/blob/master/worldedit-core/src/main/java/com/sk89q/worldedit/internal/wna/WorldNativeAccess.java

https://github.com/EngineHub/WorldEdit/blob/master/worldedit-fabric/src/main/java/com/sk89q/worldedit/fabric/internal/FabricWorldNativeAccess.java

@2No2Name 2No2Name changed the title Hopper doesn't pull items from inventory block placed without updates Hopper cache invalidation broken when inventory blocks are placed without block updates (WorldEdit) Feb 5, 2023
@2No2Name
Copy link
Member

2No2Name commented Jun 2, 2023

Also included compatibility for worledit, but there is no general solution yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed The issue has been confirmed by a project maintainer or through multiple user reports good issue report This is an example of a good issue report mod compatability This issue affects compatibility with other mods
Development

No branches or pull requests

3 participants