diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java index 7398140fb0..5ab49372a0 100644 --- a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java @@ -18,6 +18,7 @@ import java.util.Iterator; +import com.google.common.collect.Iterators; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; @@ -152,7 +153,7 @@ default boolean supportsExtraction() { * @return An iterator over the non-empty views of this storage. Calling remove on the iterator is not allowed. */ default Iterator> nonEmptyIterator() { - return TransferApiImpl.filterEmptyViews(iterator()); + return Iterators.filter(iterator(), view -> view.getAmount() > 0 && !view.isResourceBlank()); } /** diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java index 274915a556..cb8a1bd879 100644 --- a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java @@ -94,44 +94,6 @@ public T next() { }; } - public static Iterator> filterEmptyViews(Iterator> iterator) { - return new Iterator<>() { - StorageView next; - - { - findNext(); - } - - private void findNext() { - while (iterator.hasNext()) { - next = iterator.next(); - - if (next.getAmount() > 0 && !next.isResourceBlank()) { - return; - } - } - - next = null; - } - - @Override - public boolean hasNext() { - return next != null; - } - - @Override - public StorageView next() { - if (!hasNext()) { - throw new NoSuchElementException(); - } - - StorageView ret = next; - findNext(); - return ret; - } - }; - } - public static List> makeListView(SlottedStorage storage) { return new AbstractList<>() { @Override diff --git a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java index d8f9b22ac0..1f39a034f0 100644 --- a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java +++ b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java @@ -19,11 +19,15 @@ import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BUCKET; import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals; +import java.util.Iterator; + import net.minecraft.fluid.Fluids; import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant; +import net.fabricmc.fabric.api.transfer.v1.fluid.base.SingleFluidStorage; import net.fabricmc.fabric.api.transfer.v1.storage.Storage; import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil; +import net.fabricmc.fabric.api.transfer.v1.storage.StorageView; import net.fabricmc.fabric.api.transfer.v1.storage.base.FilteringStorage; import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage; import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction; @@ -31,6 +35,7 @@ public class BaseStorageTests { public static void run() { testFilteringStorage(); + testNonEmptyIteratorWithModifiedView(); } private static void testFilteringStorage() { @@ -92,4 +97,24 @@ protected boolean canInsert(FluidVariant resource) { assertEquals(BUCKET, StorageUtil.simulateExtract(storage, lava, BUCKET, null)); } + + /** + * Regression test for + * {@code nonEmptyIterator} not handling views that become empty during iteration correctly. + */ + private static void testNonEmptyIteratorWithModifiedView() { + SingleVariantStorage storage = SingleFluidStorage.withFixedCapacity(BUCKET, () -> { }); + storage.variant = FluidVariant.of(Fluids.WATER); + + Iterator> iterator = storage.nonEmptyIterator(); + storage.amount = BUCKET; + // Iterator should have a next element now + assertEquals(true, iterator.hasNext()); + assertEquals(storage, iterator.next()); + + iterator = storage.nonEmptyIterator(); + storage.amount = 0; + // Iterator should not have a next element... + assertEquals(false, iterator.hasNext()); + } }