Defer peripheral invalidation until the next tick
SquidDev opened this issue · 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 destroyed1), it just gets easier to exploit.
I think the most sensible strategy is as follows:
-
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 returnnil
.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.
-
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.
Footnotes
-
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. ↩
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.
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 9b63cc8), 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!