Merge pull request #372 from refinedmods/fix/GH-366/grid-watcher

Fix grid not handling network changes

CHANGELOG.md

@@ -22,6 +22,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ### Fixed
 
 -   Fixed missing Speed Upgrade energy usage config on Forge.
+-   Fixed Grid screen not handling network changes properly.
 
 ### Removed
 

config/checkstyle/checkstyle-suppressions.xml

@@ -9,4 +9,5 @@
     <suppress checks="FileLength" files="ModInitializer.*.java"/>
     <!-- Shadow target contains underscore -->
     <suppress checks="MemberName" files="ModelBakerImplMixin.java"/>
+    <suppress checks="HideUtilityClassConstructor" files="GridClearPacket.java"/>
 </suppressions>

refinedstorage2-grid-api/src/main/java/com/refinedmods/refinedstorage2/api/grid/GridWatcher.java

@@ -1,6 +1,5 @@
 package com.refinedmods.refinedstorage2.api.grid;
 
-import com.refinedmods.refinedstorage2.api.resource.list.ResourceListOperationResult;
 import com.refinedmods.refinedstorage2.api.storage.channel.StorageChannelType;
 import com.refinedmods.refinedstorage2.api.storage.tracked.TrackedResource;
 
@@ -25,10 +24,19 @@ public interface GridWatcher {
      *
      * @param <T>                the resource type
      * @param storageChannelType the relevant storage channel type
-     * @param change             the change
+     * @param resource           the resource
+     * @param change             the changed amount
      * @param trackedResource    the tracked resource, if present
      */
-    <T> void onChanged(StorageChannelType<T> storageChannelType,
-                       ResourceListOperationResult<T> change,
-                       @Nullable TrackedResource trackedResource);
+    <T> void onChanged(
+        StorageChannelType<T> storageChannelType,
+        T resource,
+        long change,
+        @Nullable TrackedResource trackedResource
+    );
+
+    /**
+     * Called when the grid network has been changed.
+     */
+    void onNetworkChanged();
 }

refinedstorage2-grid-api/src/main/java/com/refinedmods/refinedstorage2/api/grid/view/GridView.java

@@ -90,4 +90,9 @@ public interface GridView {
      * @return a copy of the backing list
      */
     ResourceList<Object> copyBackingList();
+
+    /**
+     * Clears the backing list, view list and tracked resources index.
+     */
+    void clear();
 }

refinedstorage2-grid-api/src/main/java/com/refinedmods/refinedstorage2/api/grid/view/GridViewImpl.java

@@ -250,4 +250,12 @@ public ResourceList<Object> copyBackingList() {
         backingList.getAll().forEach(copy::add);
         return copy;
     }
+
+    @Override
+    public void clear() {
+        backingList.clear();
+        viewListIndex.clear();
+        trackedResources.clear();
+        viewList.clear();
+    }
 }

refinedstorage2-grid-api/src/test/java/com/refinedmods/refinedstorage2/api/grid/view/GridViewImplTest.java

@@ -601,6 +601,26 @@ void shouldReuseExistingResourceWhenPreventingSortingAndRemovingExistingResource
         );
     }
 
+    @Test
+    void shouldClear() {
+        // Arrange
+        final GridView view = viewBuilder
+            .withResource("A", 15, new TrackedResource("Source", 0))
+            .withResource("B", 20, new TrackedResource("Source", 0))
+            .withResource("D", 10, new TrackedResource("Source", 0))
+            .build();
+
+        // Act
+        view.clear();
+
+        // Assert
+        assertThat(view.getViewList()).isEmpty();
+        assertThat(view.copyBackingList().getAll()).isEmpty();
+        assertThat(view.getTrackedResource("A")).isEmpty();
+        assertThat(view.getTrackedResource("B")).isEmpty();
+        assertThat(view.getTrackedResource("D")).isEmpty();
+    }
+
     private record ResourceWithMetadata(String name, int metadata) {
     }
 

refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/ConnectionProvider.java

@@ -11,5 +11,5 @@
 public interface ConnectionProvider {
     Connections findConnections(NetworkNodeContainer pivot, Set<NetworkNodeContainer> existingConnections);
 
-    List<NetworkNodeContainer> sort(Set<NetworkNodeContainer> containers);
+    List<NetworkNodeContainer> sortDeterministically(Set<NetworkNodeContainer> containers);
 }

refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/node/container/NetworkNodeContainer.java

@@ -8,4 +8,15 @@
 @FunctionalInterface
 public interface NetworkNodeContainer {
     NetworkNode getNode();
+
+    /**
+     * The container priority determines the order in which the remainder containers as the result of a network split
+     * are re-initialized with a new network.
+     * A container with the highest priority will be re-initialized first.
+     *
+     * @return the priority
+     */
+    default int getPriority() {
+        return 0;
+    }
 }

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/NetworkBuilderImpl.java

@@ -10,6 +10,7 @@
 import com.refinedmods.refinedstorage2.api.network.node.container.NetworkNodeContainer;
 
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Optional;
@@ -18,6 +19,11 @@
 import javax.annotation.Nullable;
 
 public class NetworkBuilderImpl implements NetworkBuilder {
+    private static final Comparator<NetworkNodeContainer> LOWEST_PRIORITY_FIRST = Comparator.comparingInt(
+        NetworkNodeContainer::getPriority
+    );
+    private static final Comparator<NetworkNodeContainer> HIGHEST_PRIORITY_FIRST = LOWEST_PRIORITY_FIRST.reversed();
+
     private final NetworkFactory networkFactory;
 
     public NetworkBuilderImpl(final NetworkFactory networkFactory) {
@@ -48,7 +54,7 @@ private void mergeNetworksOfNodes(final ConnectionProvider connectionProvider,
 
         final Set<Network> mergedNetworks = new HashSet<>();
 
-        for (final NetworkNodeContainer entry : foundEntries) {
+        for (final NetworkNodeContainer entry : connectionProvider.sortDeterministically(foundEntries)) {
             final NetworkNode entryNode = entry.getNode();
             final boolean isNotInPivotNetwork = !pivotNetwork.getComponent(GraphNetworkComponent.class)
                 .getContainers().contains(entry);
@@ -83,7 +89,7 @@ private Network createNetwork(final NetworkNodeContainer container) {
     private Optional<Network> findPivotNetworkForMerge(final ConnectionProvider connectionProvider,
                                                        final NetworkNodeContainer addedContainer,
                                                        final Set<NetworkNodeContainer> foundEntries) {
-        for (final NetworkNodeContainer entry : connectionProvider.sort(foundEntries)) {
+        for (final NetworkNodeContainer entry : connectionProvider.sortDeterministically(foundEntries)) {
             if (entry == addedContainer) {
                 continue;
             }
@@ -139,7 +145,7 @@ public void update(final NetworkNodeContainer container, final ConnectionProvide
     private NetworkNodeContainer findPivotNodeForRemove(final ConnectionProvider connectionProvider,
                                                         final NetworkNodeContainer removedContainer,
                                                         final Set<NetworkNodeContainer> containers) {
-        for (final NetworkNodeContainer entry : connectionProvider.sort(containers)) {
+        for (final NetworkNodeContainer entry : connectionProvider.sortDeterministically(containers)) {
             if (!entry.equals(removedContainer)) {
                 return entry;
             }
@@ -155,17 +161,18 @@ private void splitNetworks(final ConnectionProvider connectionProvider,
             throw new IllegalStateException("Network of removed node cannot be empty");
         }
 
-        removedEntries.forEach(e -> {
+        connectionProvider.sortDeterministically(removedEntries).forEach(e -> {
             if (e.getNode().getNetwork() == null) {
                 throw new IllegalStateException("Network of resulting removed node cannot be empty");
             }
             e.getNode().getNetwork().removeContainer(e);
             e.getNode().setNetwork(null);
         });
 
-        final Set<Network> networksResultingOfSplit = removedEntries
+        final Set<Network> networksResultingOfSplit = connectionProvider.sortDeterministically(removedEntries)
             .stream()
             .filter(e -> !e.equals(removedEntry))
+            .sorted(HIGHEST_PRIORITY_FIRST)
             .map(e -> {
                 final boolean establishedNetwork = initialize(e, connectionProvider);
                 if (establishedNetwork) {

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/node/container/NetworkNodeContainerPriorities.java

@@ -0,0 +1,8 @@
+package com.refinedmods.refinedstorage2.api.network.impl.node.container;
+
+public final class NetworkNodeContainerPriorities {
+    public static final int GRID = Integer.MAX_VALUE;
+
+    private NetworkNodeContainerPriorities() {
+    }
+}

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/node/container/package-info.java

@@ -0,0 +1,7 @@
+@ParametersAreNonnullByDefault
+@FieldsAndMethodsAreNonnullByDefault
+package com.refinedmods.refinedstorage2.api.network.impl.node.container;
+
+import com.refinedmods.refinedstorage2.api.core.FieldsAndMethodsAreNonnullByDefault;
+
+import javax.annotation.ParametersAreNonnullByDefault;

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/node/grid/GridNetworkNode.java

@@ -1,33 +1,33 @@
 package com.refinedmods.refinedstorage2.api.network.impl.node.grid;
 
-import com.refinedmods.refinedstorage2.api.core.CoreValidations;
 import com.refinedmods.refinedstorage2.api.grid.GridWatcher;
 import com.refinedmods.refinedstorage2.api.grid.service.GridService;
 import com.refinedmods.refinedstorage2.api.grid.service.GridServiceFactory;
 import com.refinedmods.refinedstorage2.api.grid.service.GridServiceImpl;
+import com.refinedmods.refinedstorage2.api.network.Network;
 import com.refinedmods.refinedstorage2.api.network.component.StorageNetworkComponent;
 import com.refinedmods.refinedstorage2.api.network.node.AbstractNetworkNode;
 import com.refinedmods.refinedstorage2.api.resource.ResourceAmount;
-import com.refinedmods.refinedstorage2.api.resource.list.listenable.ResourceListListener;
 import com.refinedmods.refinedstorage2.api.storage.Actor;
 import com.refinedmods.refinedstorage2.api.storage.channel.StorageChannel;
 import com.refinedmods.refinedstorage2.api.storage.channel.StorageChannelType;
 import com.refinedmods.refinedstorage2.api.storage.tracked.TrackedResource;
 
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.function.ToLongFunction;
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class GridNetworkNode extends AbstractNetworkNode implements GridServiceFactory {
+    private static final Logger LOGGER = LoggerFactory.getLogger(GridNetworkNode.class);
+
     private final Collection<? extends StorageChannelType<?>> storageChannelTypes;
-    private final Set<GridWatcher> watchers = new HashSet<>();
-    private final Map<GridWatcher, Map<StorageChannelType<?>, ResourceListListener<?>>> storageChannelListeners =
-        new HashMap<>();
+    private final Map<GridWatcher, GridWatcherRegistration> watchers = new HashMap<>();
     private final long energyUsage;
 
     public GridNetworkNode(final long energyUsage,
@@ -58,52 +58,64 @@ public long getEnergyUsage() {
     }
 
     public void addWatcher(final GridWatcher watcher, final Class<? extends Actor> actorType) {
-        CoreValidations.validateNotContains(watchers, watcher, "Watcher is already registered");
-        storageChannelTypes.forEach(storageChannelType -> attachWatcherToStorageChannel(
-            watcher,
-            actorType,
-            storageChannelType
-        ));
-        watchers.add(watcher);
+        if (watchers.containsKey(watcher)) {
+            throw new IllegalArgumentException("Watcher is already registered");
+        }
+        final GridWatcherRegistration registration = new GridWatcherRegistration(watcher, actorType);
+        attachAll(registration);
+        watchers.put(watcher, registration);
+        LOGGER.info("Added watcher {}, new count is {}", watcher, watchers.size());
     }
 
-    private <T> void attachWatcherToStorageChannel(final GridWatcher watcher,
-                                                   final Class<? extends Actor> actorType,
-                                                   final StorageChannelType<T> storageChannelType) {
-        final StorageChannel<T> storageChannel = getStorageChannel(storageChannelType);
-        final ResourceListListener<T> listener = change -> watcher.onChanged(
-            storageChannelType,
-            change,
-            storageChannel.findTrackedResourceByActorType(
-                change.resourceAmount().getResource(),
-                actorType
-            ).orElse(null)
-        );
-        storageChannel.addListener(listener);
-        storageChannelListeners.computeIfAbsent(watcher, k -> new HashMap<>()).put(storageChannelType, listener);
+    private void attachAll(final GridWatcherRegistration registration) {
+        storageChannelTypes.forEach(storageChannelType -> attach(registration, storageChannelType));
+    }
+
+    private <T> void attach(final GridWatcherRegistration registration,
+                            final StorageChannelType<T> storageChannelType) {
+        LOGGER.info("Attaching {} to {}", registration, storageChannelType);
+        registration.attach(getStorageChannel(storageChannelType), storageChannelType);
     }
 
-    @SuppressWarnings({"unchecked", "rawtypes"})
     public void removeWatcher(final GridWatcher watcher) {
-        CoreValidations.validateContains(watchers, watcher, "Watcher is not registered");
-        storageChannelListeners.get(watcher).forEach((type, listener) -> removeAttachedWatcherFromStorageChannel(
-            (StorageChannelType) type,
-            (ResourceListListener) listener
-        ));
-        storageChannelListeners.remove(watcher);
+        final GridWatcherRegistration registration = watchers.get(watcher);
+        if (registration == null) {
+            throw new IllegalArgumentException("Watcher is not registered");
+        }
+        detachAll(registration);
         watchers.remove(watcher);
+        LOGGER.info("Removed watcher {}, remaining {}", watcher, watchers.size());
     }
 
-    private <T> void removeAttachedWatcherFromStorageChannel(final StorageChannelType<T> type,
-                                                             final ResourceListListener<T> listener) {
-        final StorageChannel<T> storageChannel = getStorageChannel(type);
-        storageChannel.removeListener(listener);
+    private void detachAll(final GridWatcherRegistration registration) {
+        storageChannelTypes.forEach(storageChannelType -> detach(registration, storageChannelType));
+    }
+
+    private <T> void detach(final GridWatcherRegistration registration,
+                            final StorageChannelType<T> storageChannelType) {
+        LOGGER.info("Detaching {} from {}", registration, storageChannelType);
+        registration.detach(getStorageChannel(storageChannelType), storageChannelType);
     }
 
     @Override
     protected void onActiveChanged(final boolean newActive) {
         super.onActiveChanged(newActive);
-        watchers.forEach(watcher -> watcher.onActiveChanged(newActive));
+        watchers.keySet().forEach(watcher -> watcher.onActiveChanged(newActive));
+    }
+
+    @Override
+    public void setNetwork(@Nullable final Network network) {
+        LOGGER.info("Network has changed to {}, detaching {} watchers", network, watchers.size());
+        if (this.network != null) {
+            watchers.values().forEach(this::detachAll);
+        }
+        super.setNetwork(network);
+        if (this.network != null) {
+            watchers.forEach((watcher, registration) -> {
+                watcher.onNetworkChanged();
+                attachAll(registration);
+            });
+        }
     }
 
     @Override

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/node/grid/GridWatcherRegistration.java

@@ -0,0 +1,42 @@
+package com.refinedmods.refinedstorage2.api.network.impl.node.grid;
+
+import com.refinedmods.refinedstorage2.api.grid.GridWatcher;
+import com.refinedmods.refinedstorage2.api.resource.list.listenable.ResourceListListener;
+import com.refinedmods.refinedstorage2.api.storage.Actor;
+import com.refinedmods.refinedstorage2.api.storage.channel.StorageChannel;
+import com.refinedmods.refinedstorage2.api.storage.channel.StorageChannelType;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class GridWatcherRegistration {
+    private final GridWatcher watcher;
+    private final Class<? extends Actor> actorType;
+    private final Map<StorageChannelType<?>, ResourceListListener<?>> listeners = new HashMap<>();
+
+    public GridWatcherRegistration(final GridWatcher watcher, final Class<? extends Actor> actorType) {
+        this.watcher = watcher;
+        this.actorType = actorType;
+    }
+
+    public <T> void attach(final StorageChannel<T> storageChannel, final StorageChannelType<T> storageChannelType) {
+        final ResourceListListener<T> listener = change -> watcher.onChanged(
+            storageChannelType,
+            change.resourceAmount().getResource(),
+            change.change(),
+            storageChannel.findTrackedResourceByActorType(
+                change.resourceAmount().getResource(),
+                actorType
+            ).orElse(null)
+        );
+        storageChannel.addListener(listener);
+        listeners.put(storageChannelType, listener);
+    }
+
+    @SuppressWarnings("unchecked")
+    public <T> void detach(final StorageChannel<T> storageChannel, final StorageChannelType<T> storageChannelType) {
+        final ResourceListListener<T> listener = (ResourceListListener<T>) listeners.get(storageChannelType);
+        storageChannel.removeListener(listener);
+        listeners.remove(storageChannelType);
+    }
+}