search

CyclopsMC / CapabilityProxy

Access block capabilities from multiple sides
MIT License
5 stars 5 forks source link

Item Proxies should always trigger block updates when contents change #16

Closed AntiBlueQuirk closed 6 years ago

AntiBlueQuirk commented 6 years ago

In their current state, Item Capability Proxies can confuse other mods by changing capabilities without notifying them. An obvious consequence of this is that pipes and wires won't connect to the block unless an appropriate item is already inside. I've also found that if you place a battery in an Item Proxy and connect Ender IO conduits to it, the conduits will keep trying to charge the item even after the item is removed. (The item doesn't actually charge, but the power is wasted because the conduit thinks the block can still accept power.)

I think a simple solution is to simply trigger block updates when the block changes. This will ensure that nearby blocks get a chance to check for the new state of the block.

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

rubensworks commented 6 years ago

Thanks for the PR!

Are you sure this change actually fixes the problem (and #17)? Because last time I checked, setting the blockstate to an equal state does not actually send notifications. It's possible that this has changed by now though...

AntiBlueQuirk commented 6 years ago

Hmm, I suppose you might be right on that, since Chunk.setBlockState returns null (cancelling the update) if the state is the same. In that case, I think the only way to force an update is to either set the block state twice (to different values; not ideal), or call markAndNotifyBlock manually. (Which is a little clunky, but probably the best solution.)

AntiBlueQuirk commented 6 years ago

Alright, I've added a new version that should actually trigger block updates. I'm actually less sure that this fixes #17 now, but it should be fix this request at least. (I'm not actually sure what causes #17.) I should note that some of the other proxies (like the entity proxy) may have similar problems to this, but I'm less sure of how to fix them.

rubensworks commented 6 years ago

Could you try replacing the notification logic with this method from CyclopsCore? Using that one will make my life easier when Vanilla makes breaking changes in that scope.

Also, now that I think of it. You should definitely also report this issue to EIO, because they should check the presence of the capability before doing any operation, according to the capability provider interface contract.

AntiBlueQuirk commented 6 years ago

Yes, I suppose in the example I gave, there is clearly an EnderIO bug too, though I made this pull request for generally better behavior on this side. I'll report a bug over there too.