diff --git a/src/main/java/li/cil/oc2/api/device/Device.java b/src/main/java/li/cil/oc2/api/device/Device.java index 3ffcf1bf..e51b8d80 100644 --- a/src/main/java/li/cil/oc2/api/device/Device.java +++ b/src/main/java/li/cil/oc2/api/device/Device.java @@ -2,6 +2,7 @@ package li.cil.oc2.api.device; import li.cil.oc2.api.bus.DeviceBus; import li.cil.oc2.api.device.object.ObjectDevice; +import li.cil.oc2.api.device.provider.DeviceProvider; import java.util.List; @@ -10,8 +11,13 @@ import java.util.List; *

* The easiest and hence recommended way of implementing this interface is to use * the {@link ObjectDevice} class. + *

+ * Note that it is strongly encouraged for implementations to provide an overloaded + * {@link #equals(Object)} and {@link #hashCode()} so that identical devices can be + * detected. * * @see ObjectDevice + * @see DeviceProvider */ public interface Device { /** diff --git a/src/main/java/li/cil/oc2/api/device/IdentifiableDevice.java b/src/main/java/li/cil/oc2/api/device/IdentifiableDevice.java index 043911c4..3ae3dcbc 100644 --- a/src/main/java/li/cil/oc2/api/device/IdentifiableDevice.java +++ b/src/main/java/li/cil/oc2/api/device/IdentifiableDevice.java @@ -1,5 +1,7 @@ package li.cil.oc2.api.device; +import li.cil.oc2.api.device.provider.DeviceProvider; + import java.util.UUID; /** @@ -9,7 +11,7 @@ import java.util.UUID; * or referencing devices on a bus. Some {@link li.cil.oc2.api.bus.DeviceBusElement}s * may take care of wrapping connected devices automatically. *

- * Note that {@link li.cil.oc2.api.device.provider.DeviceProvider}s are not + * Note that {@link DeviceProvider}s are not * required to return identifiable devices. */ public interface IdentifiableDevice extends Device { @@ -19,5 +21,18 @@ public interface IdentifiableDevice extends Device { * This id must persist over save/load to prevent code in a running VM losing * track of the device. */ - UUID getUniqueId(); + UUID getUniqueIdentifier(); + + /** + * Returns a possible underlying instance of a device. + *

+ * Frequently some {@link Device} obtained from a {@link DeviceProvider} will be + * wrapped by an instance of this interface. To prevent this leading to duplicated + * device listings this allows accessing the device proper for equality checks. + * + * @return the underlying device. May be this device itself. + */ + default Device getIdentifiedDevice() { + return this; + } } diff --git a/src/main/java/li/cil/oc2/api/device/object/ObjectDevice.java b/src/main/java/li/cil/oc2/api/device/object/ObjectDevice.java index 950e2de9..c1889025 100644 --- a/src/main/java/li/cil/oc2/api/device/object/ObjectDevice.java +++ b/src/main/java/li/cil/oc2/api/device/object/ObjectDevice.java @@ -7,28 +7,27 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Objects; /** * A reflection based implementation of {@link Device} using the {@link Callback} * annotation to discover {@link DeviceMethod} in a target object via * {@link Callbacks#collectMethods(Object)}. - *

- * This class was designed targeting two possible use-cases: - *

- * The two sets of constructors are designed for these use cases, with the constructors - * targeting the workflow using an external object being {@code public}, the ones targeting - * subclassing being {@code protected}. */ -public class ObjectDevice implements Device { +public final class ObjectDevice implements Device { + private final Object object; private final ArrayList typeNames; private final List methods; private final String className; + /** + * Creates a new object device with methods in the specified object and the + * specified list of type names. + * + * @param object the object containing methods provided by this device. + * @param typeNames the type names of the device. + */ public ObjectDevice(final Object object, final List typeNames) { + this.object = object; this.typeNames = new ArrayList<>(typeNames); this.methods = Callbacks.collectMethods(object); this.className = object.getClass().getSimpleName(); @@ -38,28 +37,27 @@ public class ObjectDevice implements Device { } } + /** + * Creates a new object device with methods in the specified object and the specified + * type name. For convenience, the type name may be {@code null}, in which case using + * this constructor is equivalent to using {@link #ObjectDevice(Object)}. + * + * @param object the object containing methods provided by this device. + * @param typeName the type name of the device. + */ public ObjectDevice(final Object object, @Nullable final String typeName) { this(object, typeName != null ? Collections.singletonList(typeName) : Collections.emptyList()); } + /** + * Creates a new object device with methods in the specified object and no explicit type name. + * + * @param object the object containing the methods provided by this device. + */ public ObjectDevice(final Object object) { this(object, Collections.emptyList()); } - protected ObjectDevice(final List typeNames) { - this.typeNames = new ArrayList<>(typeNames); - this.methods = Callbacks.collectMethods(this); - this.className = getClass().getSimpleName(); - } - - protected ObjectDevice(final String typeName) { - this(Collections.singletonList(typeName)); - } - - protected ObjectDevice() { - this(Collections.emptyList()); - } - @Override public List getTypeNames() { return typeNames; @@ -71,17 +69,16 @@ public class ObjectDevice implements Device { } @Override - public boolean equals(final Object o) { + public boolean equals(@Nullable final Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; final ObjectDevice that = (ObjectDevice) o; - return typeNames.equals(that.typeNames) && - methods.equals(that.methods); + return object.equals(that.object); } @Override public int hashCode() { - return Objects.hash(typeNames, methods); + return object.hashCode(); } @Override diff --git a/src/main/java/li/cil/oc2/api/device/provider/DeviceProvider.java b/src/main/java/li/cil/oc2/api/device/provider/DeviceProvider.java index daadd017..17390bfe 100644 --- a/src/main/java/li/cil/oc2/api/device/provider/DeviceProvider.java +++ b/src/main/java/li/cil/oc2/api/device/provider/DeviceProvider.java @@ -1,6 +1,7 @@ package li.cil.oc2.api.device.provider; import li.cil.oc2.api.device.Device; +import li.cil.oc2.api.device.object.ObjectDevice; import net.minecraftforge.common.util.LazyOptional; /** @@ -14,18 +15,27 @@ import net.minecraftforge.common.util.LazyOptional; * to ensure persistence where required. Typically by returning devices that are * themselves persisted objects such as {@link net.minecraft.tileentity.TileEntity}s * or storing data in a related persisted object. + *
    + *
  • Implementations may handle multiple query types and return various device + * types depending on the query.
  • + *
  • Implementations should return the same device for the same query.
  • + *
  • + * Implementations should return either the same instance for identical + * queries or return instances that are equal to other when compared using + * {@link Object#equals(Object)} and have equal {@link Object#hashCode()}s. *

    - * Implementations may handle multiple query types and return various device - * types depending on the query. - *

    - * Implementations should return the same device for the same query. - *

    - * Implementations must return the same device type for the same query. + * This is required to avoid device duplication when a device is connected to a bus more + * than once (e.g. for blocks when connected cables are adjacent to multiple faces of the + * block). + *

  • + *
  • Implementations must return the same device type for the same query.
  • + *
*

* Providers can be registered with the IMC message {@link li.cil.oc2.api.API#IMC_ADD_DEVICE_PROVIDER}. * * @see DeviceQuery * @see BlockDeviceQuery + * @see ObjectDevice */ @FunctionalInterface public interface DeviceProvider { diff --git a/src/main/java/li/cil/oc2/common/bus/DeviceBusControllerImpl.java b/src/main/java/li/cil/oc2/common/bus/DeviceBusControllerImpl.java index adf05cda..12b17112 100644 --- a/src/main/java/li/cil/oc2/common/bus/DeviceBusControllerImpl.java +++ b/src/main/java/li/cil/oc2/common/bus/DeviceBusControllerImpl.java @@ -90,12 +90,19 @@ public class DeviceBusControllerImpl implements DeviceBusController, Steppable { @Override public void scanDevices() { devices.clear(); + + final HashMap> groupedDevices = new HashMap<>(); + for (final DeviceBusElement element : elements) { for (final IdentifiableDevice device : element.getLocalDevices()) { - final UUID uuid = device.getUniqueId(); - devices.putIfAbsent(uuid, device); + groupedDevices.computeIfAbsent(device.getIdentifiedDevice(), d -> new ArrayList<>()).add(device); } } + + for (final ArrayList group : groupedDevices.values()) { + final IdentifiableDevice device = selectDeviceDeterministically(group); + devices.putIfAbsent(device.getUniqueIdentifier(), device); + } } @Override @@ -161,7 +168,7 @@ public class DeviceBusControllerImpl implements DeviceBusController, Steppable { for (final Direction face : faces) { final LazyOptional otherCapability = tileEntity.getCapability(Capabilities.DEVICE_BUS_ELEMENT_CAPABILITY, face); otherCapability.ifPresent(otherElement -> { - final boolean isConnectedToIncomingEdge = otherElement == element; + final boolean isConnectedToIncomingEdge = Objects.equals(otherElement, element); if (!isConnectedToIncomingEdge) { return; } @@ -201,6 +208,18 @@ public class DeviceBusControllerImpl implements DeviceBusController, Steppable { writeToDevice(); } + private static IdentifiableDevice selectDeviceDeterministically(final ArrayList devices) { + IdentifiableDevice deviceWithLowestUuid = devices.get(0); + for (int i = 1; i < devices.size(); i++) { + final IdentifiableDevice device = devices.get(i); + if (device.getUniqueIdentifier().compareTo(deviceWithLowestUuid.getUniqueIdentifier()) < 0) { + deviceWithLowestUuid = device; + } + } + + return deviceWithLowestUuid; + } + private void readFromDevice() { // Only ever allow one pending message to avoid giving the VM the // power of uncontrollably inflating memory usage. Basically any diff --git a/src/main/java/li/cil/oc2/common/bus/DeviceBusElementImpl.java b/src/main/java/li/cil/oc2/common/bus/DeviceBusElementImpl.java index 2672553f..20dd783a 100644 --- a/src/main/java/li/cil/oc2/common/bus/DeviceBusElementImpl.java +++ b/src/main/java/li/cil/oc2/common/bus/DeviceBusElementImpl.java @@ -10,7 +10,7 @@ import java.util.Collection; import java.util.List; import java.util.Optional; -public class DeviceBusElementImpl implements DeviceBusElement { +public final class DeviceBusElementImpl implements DeviceBusElement { private final List devices = new ArrayList<>(); @Nullable private DeviceBusController controller; diff --git a/src/main/java/li/cil/oc2/common/device/IdentifiableDeviceImpl.java b/src/main/java/li/cil/oc2/common/device/IdentifiableDeviceImpl.java index 4279d460..a12649c4 100644 --- a/src/main/java/li/cil/oc2/common/device/IdentifiableDeviceImpl.java +++ b/src/main/java/li/cil/oc2/common/device/IdentifiableDeviceImpl.java @@ -25,10 +25,15 @@ public final class IdentifiableDeviceImpl implements IdentifiableDevice { } @Override - public UUID getUniqueId() { + public UUID getUniqueIdentifier() { return uuid; } + @Override + public Device getIdentifiedDevice() { + return device.cast().orElse(this); + } + @Override public List getTypeNames() { if (typeName != null) { @@ -51,11 +56,12 @@ public final class IdentifiableDeviceImpl implements IdentifiableDevice { if (o == null || getClass() != o.getClass()) return false; final IdentifiableDeviceImpl that = (IdentifiableDeviceImpl) o; return uuid.equals(that.uuid) && - LazyOptionalUtils.equals(device, that.device); + LazyOptionalUtils.equals(device, that.device) && + Objects.equals(typeName, that.typeName); } @Override public int hashCode() { - return Objects.hash(uuid, LazyOptionalUtils.hashCode(device)); + return Objects.hash(uuid, LazyOptionalUtils.hashCode(device), typeName); } } diff --git a/src/main/java/li/cil/oc2/common/device/provider/AbstractObjectProxy.java b/src/main/java/li/cil/oc2/common/device/provider/AbstractObjectProxy.java new file mode 100644 index 00000000..c29d1d5a --- /dev/null +++ b/src/main/java/li/cil/oc2/common/device/provider/AbstractObjectProxy.java @@ -0,0 +1,25 @@ +package li.cil.oc2.common.device.provider; + +import javax.annotation.Nullable; +import java.util.Objects; + +public abstract class AbstractObjectProxy { + protected final T value; + + public AbstractObjectProxy(final T value) { + this.value = value; + } + + @Override + public boolean equals(@Nullable final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final AbstractObjectProxy that = (AbstractObjectProxy) o; + return value.equals(that.value); + } + + @Override + public int hashCode() { + return Objects.hash(value); + } +} diff --git a/src/main/java/li/cil/oc2/common/device/provider/EnergyStorageDeviceProvider.java b/src/main/java/li/cil/oc2/common/device/provider/EnergyStorageDeviceProvider.java index bb565f29..819ed146 100644 --- a/src/main/java/li/cil/oc2/common/device/provider/EnergyStorageDeviceProvider.java +++ b/src/main/java/li/cil/oc2/common/device/provider/EnergyStorageDeviceProvider.java @@ -9,41 +9,40 @@ import net.minecraftforge.common.util.LazyOptional; import net.minecraftforge.energy.IEnergyStorage; public class EnergyStorageDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider { + private static final String ENERGY_STORAGE_TYPE_NAME = "energyStorage"; + public EnergyStorageDeviceProvider() { super(() -> Capabilities.ENERGY_STORAGE_CAPABILITY); } @Override protected LazyOptional getDevice(final BlockDeviceQuery query, final IEnergyStorage value) { - return LazyOptional.of(() -> new EnergyStorageDevice(value)); + return LazyOptional.of(() -> new ObjectDevice(new EnergyStorageDevice(value), ENERGY_STORAGE_TYPE_NAME)); } - public static final class EnergyStorageDevice extends ObjectDevice { - private final IEnergyStorage energyStorage; - + public static final class EnergyStorageDevice extends AbstractObjectProxy { public EnergyStorageDevice(final IEnergyStorage energyStorage) { - super("energyStorage"); - this.energyStorage = energyStorage; + super(energyStorage); } @Callback public int getEnergyStored() { - return energyStorage.getEnergyStored(); + return value.getEnergyStored(); } @Callback public int getMaxEnergyStored() { - return energyStorage.getMaxEnergyStored(); + return value.getMaxEnergyStored(); } @Callback public boolean canExtract() { - return energyStorage.canExtract(); + return value.canExtract(); } @Callback public boolean canReceive() { - return energyStorage.canReceive(); + return value.canReceive(); } } } diff --git a/src/main/java/li/cil/oc2/common/device/provider/FluidHandlerDeviceProvider.java b/src/main/java/li/cil/oc2/common/device/provider/FluidHandlerDeviceProvider.java index ace8f6d9..e0a9c26a 100644 --- a/src/main/java/li/cil/oc2/common/device/provider/FluidHandlerDeviceProvider.java +++ b/src/main/java/li/cil/oc2/common/device/provider/FluidHandlerDeviceProvider.java @@ -10,36 +10,35 @@ import net.minecraftforge.fluids.FluidStack; import net.minecraftforge.fluids.capability.IFluidHandler; public class FluidHandlerDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider { + private static final String FLUID_HANDLER_TYPE_NAME = "fluidHandler"; + public FluidHandlerDeviceProvider() { super(() -> Capabilities.FLUID_HANDLER_CAPABILITY); } @Override protected LazyOptional getDevice(final BlockDeviceQuery query, final IFluidHandler value) { - return LazyOptional.of(() -> new FluidHandlerDevice(value)); + return LazyOptional.of(() -> new ObjectDevice(new FluidHandlerDevice(value), FLUID_HANDLER_TYPE_NAME)); } - public static final class FluidHandlerDevice extends ObjectDevice { - private final IFluidHandler fluidHandler; - + public static final class FluidHandlerDevice extends AbstractObjectProxy { public FluidHandlerDevice(final IFluidHandler fluidHandler) { - super("fluidHandler"); - this.fluidHandler = fluidHandler; + super(fluidHandler); } @Callback public int getTanks() { - return fluidHandler.getTanks(); + return value.getTanks(); } @Callback public FluidStack getFluidInTank(final int tank) { - return fluidHandler.getFluidInTank(tank); + return value.getFluidInTank(tank); } @Callback public int getTankCapacity(final int tank) { - return fluidHandler.getTankCapacity(tank); + return value.getTankCapacity(tank); } } } diff --git a/src/main/java/li/cil/oc2/common/device/provider/ItemHandlerDeviceProvider.java b/src/main/java/li/cil/oc2/common/device/provider/ItemHandlerDeviceProvider.java index b59f3ff7..9f41f095 100644 --- a/src/main/java/li/cil/oc2/common/device/provider/ItemHandlerDeviceProvider.java +++ b/src/main/java/li/cil/oc2/common/device/provider/ItemHandlerDeviceProvider.java @@ -10,36 +10,35 @@ import net.minecraftforge.common.util.LazyOptional; import net.minecraftforge.items.IItemHandler; public class ItemHandlerDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider { + private static final String ITEM_HANDLER_TYPE_NAME = "itemHandler"; + public ItemHandlerDeviceProvider() { super(() -> Capabilities.ITEM_HANDLER_CAPABILITY); } @Override protected LazyOptional getDevice(final BlockDeviceQuery query, final IItemHandler value) { - return LazyOptional.of(() -> new ItemHandlerDevice(value)); + return LazyOptional.of(() -> new ObjectDevice(new ItemHandlerDevice(value), ITEM_HANDLER_TYPE_NAME)); } - public static final class ItemHandlerDevice extends ObjectDevice { - private final IItemHandler itemHandler; - + public static final class ItemHandlerDevice extends AbstractObjectProxy { public ItemHandlerDevice(final IItemHandler itemHandler) { - super("itemHandler"); - this.itemHandler = itemHandler; + super(itemHandler); } @Callback public int getSlots() { - return itemHandler.getSlots(); + return value.getSlots(); } @Callback public ItemStack getStackInSlot(final int slot) { - return itemHandler.getStackInSlot(slot); + return value.getStackInSlot(slot); } @Callback public int getSlotLimit(final int slot) { - return itemHandler.getSlotLimit(slot); + return value.getSlotLimit(slot); } } } diff --git a/src/main/java/li/cil/oc2/serialization/serializers/DeviceJsonSerializer.java b/src/main/java/li/cil/oc2/serialization/serializers/DeviceJsonSerializer.java index 2e6088bf..3c9c5fbb 100644 --- a/src/main/java/li/cil/oc2/serialization/serializers/DeviceJsonSerializer.java +++ b/src/main/java/li/cil/oc2/serialization/serializers/DeviceJsonSerializer.java @@ -14,7 +14,7 @@ public final class DeviceJsonSerializer implements JsonSerializer