From cdf060b274520f16513dd636ed800bc9cb9b916d Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Tue, 18 Jul 2023 13:56:03 +0200 Subject: [PATCH] A few transfer API improvements and deprecations (#3204) * A few transfer API improvements and deprecations * Forward implementation of deprecated methods --- .../api/transfer/v1/storage/Storage.java | 83 ++++++++++--------- .../api/transfer/v1/storage/StorageUtil.java | 54 ++++++++++-- .../api/transfer/v1/storage/StorageView.java | 4 +- .../gametests/VanillaStorageTests.java | 3 +- .../transfer/unittests/BaseStorageTests.java | 12 +-- .../test/transfer/unittests/FluidTests.java | 5 +- 6 files changed, 105 insertions(+), 56 deletions(-) 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 b602d1e41c..7398140fb0 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 @@ -32,11 +32,14 @@ /** * An object that can store resources. * + *

Most of the documentation that follows is quite technical. + * For an easier introduction to the API, see the wiki page. + * *

* @@ -92,17 +95,6 @@ default boolean supportsInsertion() { */ long insert(T resource, long maxAmount, TransactionContext transaction); - /** - * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state. - * The passed transaction may be null if a new transaction should be opened for the simulation. - * @see #insert - */ - default long simulateInsert(T resource, long maxAmount, @Nullable TransactionContext transaction) { - try (Transaction simulateTransaction = Transaction.openNested(transaction)) { - return insert(resource, maxAmount, simulateTransaction); - } - } - /** * Return false if calling {@link #extract} will absolutely always return 0, or true otherwise or in doubt. * @@ -123,17 +115,6 @@ default boolean supportsExtraction() { */ long extract(T resource, long maxAmount, TransactionContext transaction); - /** - * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state. - * The passed transaction may be null if a new transaction should be opened for the simulation. - * @see #extract - */ - default long simulateExtract(T resource, long maxAmount, @Nullable TransactionContext transaction) { - try (Transaction simulateTransaction = Transaction.openNested(transaction)) { - return extract(resource, maxAmount, simulateTransaction); - } - } - /** * Iterate through the contents of this storage. * Every visited {@link StorageView} represents a stored resource and an amount. @@ -187,22 +168,6 @@ default Iterable> nonEmptyViews() { return this::nonEmptyIterator; } - /** - * Return a view over this storage, for a specific resource, or {@code null} if none is quickly available. - * - *

This function should only return a non-null view if this storage can provide it quickly, - * for example with a hashmap lookup. - * If returning the requested view would require iteration through a potentially large number of views, - * {@code null} should be returned instead. - * - * @param resource The resource for which a storage view is requested. May be blank, for example to estimate capacity. - * @return A view over this storage for the passed resource, or {@code null} if none is quickly available. - */ - @Nullable - default StorageView exactView(T resource) { - return null; - } - /** * Return an integer representing the current version of this storage instance to allow for fast change detection: * if the version hasn't changed since the last time, and the storage instance is the same, the storage has the same contents. @@ -247,4 +212,44 @@ default long getVersion() { static Class> asClass() { return (Class>) (Object) Storage.class; } + + /** + * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see #insert + * @deprecated Either use transactions directly, or use {@link StorageUtil#simulateInsert}. + */ + @Deprecated(forRemoval = true) + default long simulateInsert(T resource, long maxAmount, @Nullable TransactionContext transaction) { + return StorageUtil.simulateInsert(this, resource, maxAmount, transaction); + } + + /** + * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see #extract + * @deprecated Either use transactions directly, or use {@link StorageUtil#simulateExtract}. + */ + @Deprecated(forRemoval = true) + default long simulateExtract(T resource, long maxAmount, @Nullable TransactionContext transaction) { + return StorageUtil.simulateExtract(this, resource, maxAmount, transaction); + } + + /** + * Return a view over this storage, for a specific resource, or {@code null} if none is quickly available. + * + *

This function should only return a non-null view if this storage can provide it quickly, + * for example with a hashmap lookup. + * If returning the requested view would require iteration through a potentially large number of views, + * {@code null} should be returned instead. + * + * @param resource The resource for which a storage view is requested. May be blank, for example to estimate capacity. + * @return A view over this storage for the passed resource, or {@code null} if none is quickly available. + * @deprecated Deprecated for removal without direct replacement. Use {@link #insert}, {@link #extract} or {@link #iterator} instead. + */ + @Deprecated(forRemoval = true) + @Nullable + default StorageView exactView(T resource) { + return null; + } } diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageUtil.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageUtil.java index 6bc47851a3..2c69f78f11 100644 --- a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageUtil.java +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageUtil.java @@ -93,13 +93,9 @@ public static long move(@Nullable Storage from, @Nullable Storage to, for (StorageView view : from.nonEmptyViews()) { T resource = view.getResource(); if (!filter.test(resource)) continue; - long maxExtracted; // check how much can be extracted - try (Transaction extractionTestTransaction = iterationTransaction.openNested()) { - maxExtracted = view.extract(resource, maxAmount - totalMoved, extractionTestTransaction); - extractionTestTransaction.abort(); - } + long maxExtracted = simulateExtract(view, resource, maxAmount - totalMoved, iterationTransaction); try (Transaction transferTransaction = iterationTransaction.openNested()) { // check how much can be inserted @@ -134,6 +130,52 @@ public static long move(@Nullable Storage from, @Nullable Storage to, return totalMoved; } + /** + * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see Storage#insert + */ + public static long simulateInsert(Storage storage, T resource, long maxAmount, @Nullable TransactionContext transaction) { + try (Transaction simulateTransaction = Transaction.openNested(transaction)) { + return storage.insert(resource, maxAmount, simulateTransaction); + } + } + + /** + * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see Storage#insert + */ + public static long simulateExtract(Storage storage, T resource, long maxAmount, @Nullable TransactionContext transaction) { + try (Transaction simulateTransaction = Transaction.openNested(transaction)) { + return storage.extract(resource, maxAmount, simulateTransaction); + } + } + + /** + * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see Storage#insert + */ + public static long simulateExtract(StorageView storageView, T resource, long maxAmount, @Nullable TransactionContext transaction) { + try (Transaction simulateTransaction = Transaction.openNested(transaction)) { + return storageView.extract(resource, maxAmount, simulateTransaction); + } + } + + /** + * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state. + * The passed transaction may be null if a new transaction should be opened for the simulation. + * @see Storage#insert + * @apiNote This function handles the method overload conflict for objects that implement both {@link Storage} and {@link StorageView}. + */ + // Object & is used to have a different erasure than the other overloads. + public static & StorageView> long simulateExtract(S storage, T resource, long maxAmount, @Nullable TransactionContext transaction) { + try (Transaction simulateTransaction = Transaction.openNested(transaction)) { + return storage.extract(resource, maxAmount, simulateTransaction); + } + } + /** * Try to extract any resource from a storage, up to a maximum amount. * @@ -336,7 +378,7 @@ public static ResourceAmount findExtractableContent(@Nullable Storage T extractableResource = findExtractableResource(storage, filter, transaction); if (extractableResource != null) { - long extractableAmount = storage.simulateExtract(extractableResource, Long.MAX_VALUE, transaction); + long extractableAmount = simulateExtract(storage, extractableResource, Long.MAX_VALUE, transaction); if (extractableAmount > 0) { return new ResourceAmount<>(extractableResource, extractableAmount); diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageView.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageView.java index 56a5db98fe..5761db4874 100644 --- a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageView.java +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageView.java @@ -21,7 +21,7 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext; /** - * A view of a single stored resource in a {@link Storage}, for use with {@link Storage#iterator} or {@link Storage#exactView}. + * A view of a single stored resource in a {@link Storage}, for use with {@link Storage#iterator}. * * @param The type of the stored resource. * @@ -57,7 +57,7 @@ public interface StorageView { /** * @return The total amount of {@link #getResource} that could be stored in this view, - * or an estimate of the number of resources that could be stored if this view has a blank resource. + * or an estimated upper bound on the number of resources that could be stored if this view has a blank resource. */ long getCapacity(); diff --git a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java index 97ee26ceca..3080a914f7 100644 --- a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java +++ b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java @@ -45,6 +45,7 @@ import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant; import net.fabricmc.fabric.api.transfer.v1.storage.SlottedStorage; 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.transaction.Transaction; import net.fabricmc.fabric.test.transfer.mixin.AbstractFurnaceBlockEntityAccessor; @@ -224,7 +225,7 @@ public void testShulkerNoInsert(TestContext context) { ShulkerBoxBlockEntity shulker = (ShulkerBoxBlockEntity) context.getBlockEntity(pos); InventoryStorage storage = InventoryStorage.of(shulker, null); - if (storage.simulateInsert(ItemVariant.of(Items.SHULKER_BOX), 1, null) > 0) { + if (StorageUtil.simulateInsert(storage, ItemVariant.of(Items.SHULKER_BOX), 1, null) > 0) { context.throwPositionedException("Expected shulker box to be rejected", pos); } 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 1c0a9fbd04..d8f9b22ac0 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 @@ -66,9 +66,9 @@ protected boolean canInsert(FluidVariant resource) { } // Insertion through the filter should fail. - assertEquals(0L, noWater.simulateInsert(water, BUCKET, null)); + assertEquals(0L, StorageUtil.simulateInsert(noWater, water, BUCKET, null)); // Extraction should also fail. - assertEquals(0L, noWater.simulateExtract(water, BUCKET, null)); + assertEquals(0L, StorageUtil.simulateExtract(noWater, water, BUCKET, null)); // The fluid should be visible. assertEquals(water, StorageUtil.findStoredResource(noWater)); // Test the filter. @@ -83,13 +83,13 @@ protected boolean canInsert(FluidVariant resource) { // Lava insertion and extract should proceed just fine. try (Transaction tx = Transaction.openOuter()) { assertEquals(BUCKET, noWater.insert(lava, BUCKET, tx)); - assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx)); + assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx)); // Test that simulating doesn't change the state... - assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx)); - assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx)); + assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx)); + assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx)); tx.commit(); } - assertEquals(BUCKET, storage.simulateExtract(lava, BUCKET, null)); + assertEquals(BUCKET, StorageUtil.simulateExtract(storage, lava, BUCKET, null)); } } diff --git a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/FluidTests.java b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/FluidTests.java index b2d57ebbfe..42f7040efe 100644 --- a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/FluidTests.java +++ b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/FluidTests.java @@ -22,6 +22,7 @@ import net.minecraft.nbt.NbtCompound; import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant; +import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil; import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage; import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage; import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction; @@ -78,7 +79,7 @@ private static void testFluidStorage() { // Should not allow lava (canInsert returns false) if (waterStorage.insert(LAVA, BUCKET, tx) != 0) throw new AssertionError("Lava inserted"); // Should allow insert, but without mutating the storage. - if (waterStorage.simulateInsert(WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Simulated insert failed"); + if (StorageUtil.simulateInsert(waterStorage, WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Simulated insert failed"); // Should allow insert if (waterStorage.insert(TAGGED_WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 1 failed"); // Variants are different, should not allow insert @@ -90,7 +91,7 @@ private static void testFluidStorage() { // Should allow extraction if (waterStorage.extract(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Extraction failed"); // Simulated extraction should succeed but do nothing - if (waterStorage.simulateExtract(TAGGED_WATER, Long.MAX_VALUE, tx) != BUCKET) throw new AssertionError("Simulated extraction failed"); + if (StorageUtil.simulateExtract(waterStorage, TAGGED_WATER, Long.MAX_VALUE, tx) != BUCKET) throw new AssertionError("Simulated extraction failed"); // Re-insert if (waterStorage.insert(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 3 failed"); // Test contents