From 0c271732cb8ac4b6b8ee42d83eb2d5e0672c0d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20N=C3=BCcke?= Date: Tue, 25 Jan 2022 19:57:43 +0100 Subject: [PATCH] Add option to clean up stuff (via unmount) when devices go missing. For block devices, this can happen when the block is broken while the computer is unloaded, for example. For items this is more esoteric, but it theoretically means items disappear while the container managing the item as a device is unloaded. --- .../device/provider/BlockDeviceProvider.java | 23 ++++++++ .../device/provider/ItemDeviceProvider.java | 21 +++++++ .../blockentity/ComputerBlockEntity.java | 2 +- .../bus/AbstractGroupingDeviceBusElement.java | 58 ++++++++++++++----- .../bus/BlockEntityDeviceBusElement.java | 48 +++++++++++++-- .../bus/ItemHandlerDeviceBusElement.java | 43 +++++++++++++- .../item/AbstractBlockDeviceVMDevice.java | 7 +++ .../block/DiskDriveDeviceProvider.java | 6 ++ .../item/HardDriveItemDeviceProvider.java | 10 ++++ ...iveWithExternalDataItemDeviceProvider.java | 10 ++++ 10 files changed, 202 insertions(+), 26 deletions(-) diff --git a/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java b/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java index 3a66ee39..b4abef3b 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java +++ b/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java @@ -1,7 +1,10 @@ package li.cil.oc2.api.bus.device.provider; import li.cil.oc2.api.bus.device.Device; +import li.cil.oc2.api.bus.device.rpc.RPCDevice; +import li.cil.oc2.api.bus.device.vm.VMDevice; import li.cil.oc2.api.util.Invalidatable; +import net.minecraft.nbt.CompoundTag; import net.minecraftforge.registries.IForgeRegistryEntry; /** @@ -48,4 +51,24 @@ public interface BlockDeviceProvider extends IForgeRegistryEntry getDevice(BlockDeviceQuery query); + + /** + * Last-resort cleanup method for devices provided by this provider. + *

+ * This is the equivalent of {@link RPCDevice#unmount()} or {@link VMDevice#unmount()}, + * for devices that have gone missing unexpectedly, so this method could no longer be + * called on the actual device. + *

+ * For block devices, this can happen if the block the device was created for has been + * removed while the connected computer was unloaded, or the cable connecting the block + * the device was provided for with the computer was broken while the computer was unloaded. + *

+ * Implementing this is only necessary, if the device holds some out-of-NBT serialized + * data, or does something similar. + * + * @param query the query that resulted in the device when it still existed, if available. + * @param tag data last serialized by the device that went missing. + */ + default void unmount(final BlockDeviceQuery query, final CompoundTag tag) { + } } diff --git a/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java b/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java index a50dfa75..6cc6a681 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java +++ b/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java @@ -1,6 +1,9 @@ package li.cil.oc2.api.bus.device.provider; import li.cil.oc2.api.bus.device.ItemDevice; +import li.cil.oc2.api.bus.device.rpc.RPCDevice; +import li.cil.oc2.api.bus.device.vm.VMDevice; +import net.minecraft.nbt.CompoundTag; import net.minecraftforge.registries.IForgeRegistryEntry; import java.util.Optional; @@ -57,4 +60,22 @@ public interface ItemDeviceProvider extends IForgeRegistryEntry + * This is the equivalent of {@link RPCDevice#unmount()} or {@link VMDevice#unmount()}, + * for devices that have gone missing unexpectedly, so this method could no longer be + * called on the actual device. + *

+ * For item devices this is rather unlikely. It means an item disappeared while the + * block managing the item device was unloaded. + *

+ * Implementing this is only necessary, if the device holds some out-of-NBT serialized + * data, or does something similar. + * + * @param tag the data last serialized by the device went missing. + */ + default void unmount(final CompoundTag tag) { + } } diff --git a/src/main/java/li/cil/oc2/common/blockentity/ComputerBlockEntity.java b/src/main/java/li/cil/oc2/common/blockentity/ComputerBlockEntity.java index c43a8d5e..ea3ca4aa 100644 --- a/src/main/java/li/cil/oc2/common/blockentity/ComputerBlockEntity.java +++ b/src/main/java/li/cil/oc2/common/blockentity/ComputerBlockEntity.java @@ -335,7 +335,7 @@ public final class ComputerBlockEntity extends ModBlockEntity implements Termina public void addOwnDevices() { assert level != null; - for (final BlockEntry info : collectDevices(level, getPosition(), null)) { + for (final BlockEntry info : collectDevices(level, getPosition(), null).getEntries()) { devices.add(info.getDevice()); super.addDevice(info.getDevice()); } diff --git a/src/main/java/li/cil/oc2/common/bus/AbstractGroupingDeviceBusElement.java b/src/main/java/li/cil/oc2/common/bus/AbstractGroupingDeviceBusElement.java index 36b21d19..a89673db 100644 --- a/src/main/java/li/cil/oc2/common/bus/AbstractGroupingDeviceBusElement.java +++ b/src/main/java/li/cil/oc2/common/bus/AbstractGroupingDeviceBusElement.java @@ -5,13 +5,21 @@ import li.cil.oc2.common.util.NBTTagIds; import net.minecraft.nbt.CompoundTag; import net.minecraft.nbt.ListTag; +import javax.annotation.Nullable; import java.util.*; -public abstract class AbstractGroupingDeviceBusElement extends AbstractDeviceBusElement { +public abstract class AbstractGroupingDeviceBusElement extends AbstractDeviceBusElement { private static final String GROUPS_TAG_NAME = "groups"; private static final String GROUP_ID_TAG_NAME = "groupId"; private static final String GROUP_DATA_TAG_NAME = "groupData"; + protected abstract class QueryResult { + @Nullable + public abstract TQuery getQuery(); + + public abstract Set getEntries(); + } + protected interface Entry { Optional getDeviceDataKey(); @@ -23,7 +31,7 @@ public abstract class AbstractGroupingDeviceBusElement> groups; + protected final ArrayList> groups; /////////////////////////////////////////////////////////////////// @@ -80,7 +88,7 @@ public abstract class AbstractGroupingDeviceBusElement { if (devicesTag.contains(key, NBTTagIds.TAG_COMPOUND)) { @@ -94,8 +102,8 @@ public abstract class AbstractGroupingDeviceBusElement getDeviceIdentifier(final Device device) { for (int i = 0; i < groupCount; i++) { - final HashSet group = groups.get(i); - for (final T deviceInfo : group) { + final HashSet group = groups.get(i); + for (final TEntry deviceInfo : group) { if (Objects.equals(device, deviceInfo.getDevice())) { return Optional.of(groupIds[i]); } @@ -106,22 +114,23 @@ public abstract class AbstractGroupingDeviceBusElement newEntries) { - final HashSet oldEntries = groups.get(index); + protected final void setEntriesForGroup(final int index, final QueryResult queryResult) { + final Set newEntries = queryResult.getEntries(); + final HashSet oldEntries = groups.get(index); if (Objects.equals(newEntries, oldEntries)) { return; } - final HashSet removedEntries = new HashSet<>(oldEntries); + final HashSet removedEntries = new HashSet<>(oldEntries); removedEntries.removeAll(newEntries); - for (final T entry : removedEntries) { + for (final TEntry entry : removedEntries) { devices.removeInt(entry.getDevice()); onEntryRemoved(entry); } - final HashSet addedEntries = new HashSet<>(newEntries); + final HashSet addedEntries = new HashSet<>(newEntries); addedEntries.removeAll(oldEntries); - for (final T entry : addedEntries) { + for (final TEntry entry : addedEntries) { devices.put(entry.getDevice(), entry.getDeviceEnergyConsumption().orElse(0)); onEntryAdded(entry); } @@ -130,31 +139,48 @@ public abstract class AbstractGroupingDeviceBusElement invalidDataKeys = new HashSet<>(devicesTag.getAllKeys()); + for (final TEntry entry : addedEntries) { entry.getDeviceDataKey().ifPresent(key -> { + invalidDataKeys.remove(key); if (devicesTag.contains(key, NBTTagIds.TAG_COMPOUND)) { entry.getDevice().deserializeNBT(devicesTag.getCompound(key)); + } else { + devicesTag.remove(key); } }); } + final TQuery query = queryResult.getQuery(); + for (final String invalidDataKey : invalidDataKeys) { + if (devicesTag.contains(invalidDataKey, NBTTagIds.TAG_COMPOUND)) { + final CompoundTag tag = devicesTag.getCompound(invalidDataKey); + onEntryRemoved(invalidDataKey, tag, query); + } + devicesTag.remove(invalidDataKey); + } + scanDevices(); } - protected void onEntryRemoved(final T entry) { + protected void onEntryAdded(final TEntry entry) { } - protected void onEntryAdded(final T entry) { + protected void onEntryRemoved(final TEntry entry) { + } + + protected void onEntryRemoved(final String dataKey, final CompoundTag data, @Nullable final TQuery query) { } /////////////////////////////////////////////////////////////////// private void saveGroup(final int index) { final CompoundTag devicesTag = new CompoundTag(); - for (final T entry : groups.get(index)) { + for (final TEntry entry : groups.get(index)) { entry.getDeviceDataKey().ifPresent(key -> { final CompoundTag deviceTag = entry.getDevice().serializeNBT(); if (!deviceTag.isEmpty()) { diff --git a/src/main/java/li/cil/oc2/common/bus/BlockEntityDeviceBusElement.java b/src/main/java/li/cil/oc2/common/bus/BlockEntityDeviceBusElement.java index 9897a6e4..44ca2514 100644 --- a/src/main/java/li/cil/oc2/common/bus/BlockEntityDeviceBusElement.java +++ b/src/main/java/li/cil/oc2/common/bus/BlockEntityDeviceBusElement.java @@ -4,9 +4,11 @@ import li.cil.oc2.api.bus.BlockDeviceBusElement; import li.cil.oc2.api.bus.DeviceBus; import li.cil.oc2.api.bus.DeviceBusElement; import li.cil.oc2.api.bus.device.Device; +import li.cil.oc2.api.bus.device.provider.BlockDeviceProvider; import li.cil.oc2.api.bus.device.provider.BlockDeviceQuery; import li.cil.oc2.api.util.Invalidatable; import li.cil.oc2.common.Constants; +import li.cil.oc2.common.bus.device.provider.Providers; import li.cil.oc2.common.bus.device.rpc.TypeNameRPCDevice; import li.cil.oc2.common.bus.device.util.BlockDeviceInfo; import li.cil.oc2.common.bus.device.util.Devices; @@ -15,11 +17,14 @@ import li.cil.oc2.common.util.LevelUtils; import li.cil.oc2.common.util.ServerScheduler; import net.minecraft.core.BlockPos; import net.minecraft.core.Direction; +import net.minecraft.nbt.CompoundTag; +import net.minecraft.resources.ResourceLocation; import net.minecraft.world.level.ChunkPos; import net.minecraft.world.level.Level; import net.minecraft.world.level.LevelAccessor; import net.minecraft.world.level.block.entity.BlockEntity; import net.minecraftforge.common.util.LazyOptional; +import net.minecraftforge.registries.IForgeRegistry; import javax.annotation.Nullable; import java.util.*; @@ -27,7 +32,7 @@ import java.util.*; import static java.util.Objects.requireNonNull; import static li.cil.oc2.common.util.RegistryUtils.optionalKey; -public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElement implements BlockDeviceBusElement { +public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElement implements BlockDeviceBusElement { private final BlockEntity blockEntity; /////////////////////////////////////////////////////////////////// @@ -96,10 +101,10 @@ public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElemen return; } - final HashSet newDevices = collectDevices(level, pos, direction); + final BlockQueryResult queryResult = collectDevices(level, pos, direction); final int index = direction.get3DDataValue(); - setEntriesForGroup(index, newDevices); + setEntriesForGroup(index, queryResult); } public void initialize() { @@ -124,10 +129,11 @@ public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElemen return canScanContinueTowards(direction); } - protected HashSet collectDevices(final Level level, final BlockPos pos, @Nullable final Direction direction) { + protected BlockQueryResult collectDevices(final Level level, final BlockPos pos, @Nullable final Direction direction) { + final BlockDeviceQuery query = Devices.makeQuery(level, pos, direction != null ? direction.getOpposite() : null); final HashSet entries = new HashSet<>(); + if (canDetectDevicesTowards(direction)) { - final BlockDeviceQuery query = Devices.makeQuery(level, pos, direction != null ? direction.getOpposite() : null); for (final Invalidatable deviceInfo : Devices.getDevices(query)) { if (deviceInfo.isPresent()) { entries.add(new BlockEntry(deviceInfo, pos)); @@ -137,7 +143,7 @@ public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElemen collectSyntheticDevices(level, pos, direction, entries); - return entries; + return new BlockQueryResult(query, entries); } protected void collectSyntheticDevices(final Level level, final BlockPos pos, @Nullable final Direction direction, final HashSet entries) { @@ -159,6 +165,17 @@ public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElemen entry.removeListener(); } + @Override + protected void onEntryRemoved(final String dataKey, final CompoundTag tag, @Nullable final BlockDeviceQuery query) { + assert query != null : "Passed null query for block device bus element."; + super.onEntryRemoved(dataKey, tag, query); + final IForgeRegistry registry = Providers.BLOCK_DEVICE_PROVIDER_REGISTRY.get(); + final BlockDeviceProvider provider = registry.getValue(new ResourceLocation(dataKey)); + if (provider != null) { + provider.unmount(query, tag); + } + } + /////////////////////////////////////////////////////////////////// private void scanNeighborsForDevices() { @@ -185,6 +202,25 @@ public class BlockEntityDeviceBusElement extends AbstractGroupingDeviceBusElemen /////////////////////////////////////////////////////////////////// + protected final class BlockQueryResult extends QueryResult { + private final BlockDeviceQuery query; + private final Set entries; + + public BlockQueryResult(final BlockDeviceQuery query, final Set entries) { + this.query = query; + this.entries = entries; + } + + public BlockDeviceQuery getQuery() { + return query; + } + + @Override + public Set getEntries() { + return entries; + } + } + protected final class BlockEntry implements Entry { private final Invalidatable deviceInfo; @Nullable private final String dataKey; diff --git a/src/main/java/li/cil/oc2/common/bus/ItemHandlerDeviceBusElement.java b/src/main/java/li/cil/oc2/common/bus/ItemHandlerDeviceBusElement.java index 7d09f669..80384cb4 100644 --- a/src/main/java/li/cil/oc2/common/bus/ItemHandlerDeviceBusElement.java +++ b/src/main/java/li/cil/oc2/common/bus/ItemHandlerDeviceBusElement.java @@ -2,8 +2,10 @@ package li.cil.oc2.common.bus; import li.cil.oc2.api.bus.device.Device; import li.cil.oc2.api.bus.device.ItemDevice; +import li.cil.oc2.api.bus.device.provider.ItemDeviceProvider; import li.cil.oc2.api.bus.device.provider.ItemDeviceQuery; import li.cil.oc2.api.bus.device.rpc.RPCDevice; +import li.cil.oc2.common.bus.device.provider.Providers; import li.cil.oc2.common.bus.device.rpc.TypeNameRPCDevice; import li.cil.oc2.common.bus.device.util.Devices; import li.cil.oc2.common.bus.device.util.ItemDeviceInfo; @@ -12,13 +14,15 @@ import li.cil.oc2.common.util.NBTTagIds; import net.minecraft.nbt.CompoundTag; import net.minecraft.resources.ResourceLocation; import net.minecraft.world.item.ItemStack; +import net.minecraftforge.registries.IForgeRegistry; +import javax.annotation.Nullable; import java.util.*; import java.util.function.Function; import static li.cil.oc2.common.util.RegistryUtils.optionalKey; -public final class ItemHandlerDeviceBusElement extends AbstractGroupingDeviceBusElement { +public final class ItemHandlerDeviceBusElement extends AbstractGroupingDeviceBusElement { private final Function queryFactory; public ItemHandlerDeviceBusElement(final int slotCount, final Function queryFactory) { @@ -44,9 +48,9 @@ public final class ItemHandlerDeviceBusElement extends AbstractGroupingDeviceBus final HashSet newDevices = new HashSet<>(Devices.getDevices(query).stream().map(ItemEntry::new).toList()); insertItemNameDevice(stack, newDevices); importDeviceDataFromItemStack(stack, newDevices); - setEntriesForGroup(slot, newDevices); + setEntriesForGroup(slot, new ItemQueryResult(query, newDevices)); } else { - setEntriesForGroup(slot, Collections.emptySet()); + setEntriesForGroup(slot, new ItemQueryResult(null, Collections.emptySet())); } } @@ -73,6 +77,18 @@ public final class ItemHandlerDeviceBusElement extends AbstractGroupingDeviceBus /////////////////////////////////////////////////////////////////// + @Override + protected void onEntryRemoved(final String dataKey, final CompoundTag tag, @Nullable final ItemDeviceQuery query) { + super.onEntryRemoved(dataKey, tag, query); + final IForgeRegistry registry = Providers.ITEM_DEVICE_PROVIDER_REGISTRY.get(); + final ItemDeviceProvider provider = registry.getValue(new ResourceLocation(dataKey)); + if (provider != null) { + provider.unmount(query, tag); + } + } + + /////////////////////////////////////////////////////////////////// + private void importDeviceDataFromItemStack(final ItemStack stack, final HashSet entries) { final CompoundTag exportedTag = ItemDeviceUtils.getItemDeviceData(stack); if (!exportedTag.isEmpty()) { @@ -97,6 +113,27 @@ public final class ItemHandlerDeviceBusElement extends AbstractGroupingDeviceBus /////////////////////////////////////////////////////////////////// + protected final class ItemQueryResult extends QueryResult { + @Nullable private final ItemDeviceQuery query; + private final Set entries; + + public ItemQueryResult(@Nullable final ItemDeviceQuery query, final Set entries) { + this.query = query; + this.entries = entries; + } + + @Nullable + @Override + public ItemDeviceQuery getQuery() { + return query; + } + + @Override + public Set getEntries() { + return entries; + } + } + protected record ItemEntry(ItemDeviceInfo deviceInfo) implements Entry { @Override public Optional getDeviceDataKey() { diff --git a/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java b/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java index 77a0268b..0eded280 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java +++ b/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java @@ -151,6 +151,13 @@ public abstract class AbstractBlockDeviceVMDevice getBlockDevice(final BlockDeviceQuery query, final DiskDriveBlockEntity blockEntity) { // We only allow connecting to exactly one face of the disk drive to ensure only one diff --git a/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveItemDeviceProvider.java b/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveItemDeviceProvider.java index 9d441947..7068213e 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveItemDeviceProvider.java +++ b/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveItemDeviceProvider.java @@ -8,9 +8,11 @@ import li.cil.oc2.common.bus.device.item.HardDriveVMDevice; import li.cil.oc2.common.bus.device.provider.util.AbstractItemDeviceProvider; import li.cil.oc2.common.item.HardDriveItem; import li.cil.oc2.common.util.LocationSupplierUtils; +import net.minecraft.nbt.CompoundTag; import net.minecraft.util.Mth; import net.minecraft.world.item.ItemStack; +import javax.annotation.Nullable; import java.util.Optional; public final class HardDriveItemDeviceProvider extends AbstractItemDeviceProvider { @@ -20,6 +22,14 @@ public final class HardDriveItemDeviceProvider extends AbstractItemDeviceProvide /////////////////////////////////////////////////////////////////// + @Override + public void unmount(@Nullable final ItemDeviceQuery query, final CompoundTag tag) { + super.unmount(query, tag); + HardDriveVMDevice.unmount(tag); + } + + /////////////////////////////////////////////////////////////////// + @Override protected Optional getItemDevice(final ItemDeviceQuery query) { return Optional.of(new HardDriveVMDevice(query.getItemStack(), getCapacity(query), false, LocationSupplierUtils.of(query))); diff --git a/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveWithExternalDataItemDeviceProvider.java b/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveWithExternalDataItemDeviceProvider.java index 9a2ffd0c..d45e854f 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveWithExternalDataItemDeviceProvider.java +++ b/src/main/java/li/cil/oc2/common/bus/device/provider/item/HardDriveWithExternalDataItemDeviceProvider.java @@ -9,8 +9,10 @@ import li.cil.oc2.common.bus.device.item.HardDriveVMDeviceWithInitialData; import li.cil.oc2.common.bus.device.provider.util.AbstractItemDeviceProvider; import li.cil.oc2.common.item.HardDriveWithExternalDataItem; import li.cil.oc2.common.util.LocationSupplierUtils; +import net.minecraft.nbt.CompoundTag; import net.minecraft.world.item.ItemStack; +import javax.annotation.Nullable; import java.util.Optional; public final class HardDriveWithExternalDataItemDeviceProvider extends AbstractItemDeviceProvider { @@ -20,6 +22,14 @@ public final class HardDriveWithExternalDataItemDeviceProvider extends AbstractI /////////////////////////////////////////////////////////////////// + @Override + public void unmount(@Nullable final ItemDeviceQuery query, final CompoundTag tag) { + super.unmount(query, tag); + HardDriveVMDeviceWithInitialData.unmount(tag); + } + + /////////////////////////////////////////////////////////////////// + @Override protected Optional getItemDevice(final ItemDeviceQuery query) { final ItemStack stack = query.getItemStack();