CC-Tweaked icon indicating copy to clipboard operation
CC-Tweaked copied to clipboard

Defer peripheral invalidation until the next tick

Open SquidDev opened this issue 2 years ago • 2 comments

@baeuric implemented this in #780 in order to fix #696. At the time I left it because I'm not entirely comfortable with how safe it is. On the back of #882, I think we need to implement this.

The obvious issue here is that peripherals could be usable for a single tick after their tile has removed from the world. This is already a problem (main thread tasks are run even if a peripheral is destroyed[^1]), it just gets easier to exploit.

I think the most sensible strategy is as follows:

  • [x] Merge in #780, or some variant of it.

  • Peripheral calls which run on the computer thread are kept as is. The peripheral will not have been detached at this point and, as peripherals shouldn't be interacting with the world thread, this is hopefully safe.

  • [ ] Peripherals are given a custom ILuaContext which overrides the various main thread enqueue functions. The provided runnable will be wrapped to ensure the underlying peripheral hasn't been detached - if so we error or return nil.

    We need to be careful here that we don't have too many false positives. We don't want a repeat of Plethora's "Block is no longer here". We should at least check what happens with furnaces.

  • [x] Inventory and fluid transfer methods should check the underlying peripheral target hasn't been removed (assuming it's a BlockEntity).

We're going to need to do a lot of testing here, as I'm sure there's some side effects I'm not aware of. I think this is a change worth running on @Wojbie's server if they're willing.

[^1]: This probably leads to dupe bugs. I've definitely manage to destroy items this way by piping them into a broken chest, so the inverse is probably true too.

SquidDev avatar Aug 16 '21 08:08 SquidDev

Taking this off 1.99 for now as I think the previous changes are going to cause problems enough - will let them sit for one release before breaking anything else.

SquidDev avatar Nov 28 '21 20:11 SquidDev

One thing we might want to retry peripheral methods on the new peripheral if they have the same type. There's definitely going to be cases where the peripheral gets erroneously reattached (such as in 9b63cc81b1765d566e8ce2872b655cd5712194ca), and it'd be nice to handle that gracefully.

To do this, we should probably hoist-out the main-thread wrapper into its own class, so we can do an instanceof check, and have some custom logic. I'm not quite sure how this looks for generic peripherals though, maybe we need another rewrite there!

SquidDev avatar Mar 21 '24 20:03 SquidDev