Deduplicating devices on a bus.

This commit is contained in:
Florian Nücke
2020-12-01 19:54:49 +01:00
parent cf6efff5e3
commit 796ca0e0d1
14 changed files with 151 additions and 76 deletions

View File

@@ -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;
* <p>
* The easiest and hence recommended way of implementing this interface is to use
* the {@link ObjectDevice} class.
* <p>
* 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 {
/**

View File

@@ -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.
* <p>
* Note that {@link li.cil.oc2.api.device.provider.DeviceProvider}s are <em>not</em>
* Note that {@link DeviceProvider}s are <em>not</em>
* 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.
* <p>
* 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;
}
}

View File

@@ -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)}.
* <p>
* This class was designed targeting two possible use-cases:
* <ul>
* <li>Wrapping some separate object containing the annotated method.</li>
* <li>Subclassing this type and implementing annotated methods in the subclass.</li>
* </ul>
* 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<String> typeNames;
private final List<DeviceMethod> 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<String> 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<String> 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<String> 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

View File

@@ -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.
* <ul>
* <li>Implementations <em>may</em> handle multiple query types and return various device
* types depending on the query.</li>
* <li>Implementations <em>should</em> return the same device for the same query.</li>
* <li>
* Implementations <em>should</em> 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.
* <p>
* Implementations <em>may</em> handle multiple query types and return various device
* types depending on the query.
* <p>
* Implementations <em>should</em> return the same device for the same query.
* <p>
* Implementations <em>must</em> 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).
* </li>
* <li>Implementations <em>must</em> return the same device type for the same query.</li>
* </ul>
* <p>
* 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 {

View File

@@ -90,12 +90,19 @@ public class DeviceBusControllerImpl implements DeviceBusController, Steppable {
@Override
public void scanDevices() {
devices.clear();
final HashMap<Device, ArrayList<IdentifiableDevice>> 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<IdentifiableDevice> 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<DeviceBusElement> 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<IdentifiableDevice> 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

View File

@@ -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<IdentifiableDevice> devices = new ArrayList<>();
@Nullable private DeviceBusController controller;

View File

@@ -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.<Device>cast().orElse(this);
}
@Override
public List<String> 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);
}
}

View File

@@ -0,0 +1,25 @@
package li.cil.oc2.common.device.provider;
import javax.annotation.Nullable;
import java.util.Objects;
public abstract class AbstractObjectProxy<T> {
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);
}
}

View File

@@ -9,41 +9,40 @@ import net.minecraftforge.common.util.LazyOptional;
import net.minecraftforge.energy.IEnergyStorage;
public class EnergyStorageDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider<IEnergyStorage> {
private static final String ENERGY_STORAGE_TYPE_NAME = "energyStorage";
public EnergyStorageDeviceProvider() {
super(() -> Capabilities.ENERGY_STORAGE_CAPABILITY);
}
@Override
protected LazyOptional<Device> 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<IEnergyStorage> {
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();
}
}
}

View File

@@ -10,36 +10,35 @@ import net.minecraftforge.fluids.FluidStack;
import net.minecraftforge.fluids.capability.IFluidHandler;
public class FluidHandlerDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider<IFluidHandler> {
private static final String FLUID_HANDLER_TYPE_NAME = "fluidHandler";
public FluidHandlerDeviceProvider() {
super(() -> Capabilities.FLUID_HANDLER_CAPABILITY);
}
@Override
protected LazyOptional<Device> 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<IFluidHandler> {
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);
}
}
}

View File

@@ -10,36 +10,35 @@ import net.minecraftforge.common.util.LazyOptional;
import net.minecraftforge.items.IItemHandler;
public class ItemHandlerDeviceProvider extends AbstractCapabilityAnyTileEntityDeviceProvider<IItemHandler> {
private static final String ITEM_HANDLER_TYPE_NAME = "itemHandler";
public ItemHandlerDeviceProvider() {
super(() -> Capabilities.ITEM_HANDLER_CAPABILITY);
}
@Override
protected LazyOptional<Device> 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<IItemHandler> {
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);
}
}
}

View File

@@ -14,7 +14,7 @@ public final class DeviceJsonSerializer implements JsonSerializer<IdentifiableDe
}
final JsonObject deviceJson = new JsonObject();
deviceJson.add("deviceId", context.serialize(src.getUniqueId()));
deviceJson.add("deviceId", context.serialize(src.getUniqueIdentifier()));
deviceJson.add("typeNames", context.serialize(src.getTypeNames()));
final JsonArray methodsJson = new JsonArray();

View File

@@ -67,7 +67,7 @@ public class DeviceBusTests {
final IdentifiableDevice device = mock(IdentifiableDevice.class);
when(busElement.getLocalDevices()).thenReturn(Collections.singletonList(device));
when(device.getUniqueId()).thenReturn(UUID.randomUUID());
when(device.getUniqueIdentifier()).thenReturn(UUID.randomUUID());
Assertions.assertEquals(DeviceBusControllerImpl.State.READY,
controller.scan(world, CONTROLLER_POS));

View File

@@ -130,7 +130,7 @@ public class ObjectDeviceProtocolTests {
final JsonObject request = new JsonObject();
request.addProperty("type", "invoke");
final JsonObject methodInvocation = new JsonObject();
methodInvocation.addProperty("deviceId", device.getUniqueId().toString());
methodInvocation.addProperty("deviceId", device.getUniqueIdentifier().toString());
methodInvocation.addProperty("name", name);
final JsonArray parametersJson = new JsonArray();
methodInvocation.add("parameters", parametersJson);
@@ -263,7 +263,7 @@ public class ObjectDeviceProtocolTests {
}
@Override
public UUID getUniqueId() {
public UUID getUniqueIdentifier() {
return UUID;
}
}