Merge pull request #140 from Kneelawk/update/fapi-0.91.0+1.20.1

Update to FAPI `0.91.0+1.20.1`

fabric-game-rule-api-v1/src/main/java/net/fabricmc/fabric/api/gamerule/v1/rule/DoubleRule.java

@@ -114,7 +114,12 @@ public boolean validate(String value) {
 		try {
 			final double d = Double.parseDouble(value);
 
-			return this.inBounds(d);
+			if (!this.inBounds(d)) {
+				return false;
+			}
+
+			this.value = d;
+			return true;
 		} catch (NumberFormatException ignored) {
 			return false;
 		}

fabric-game-rule-api-v1/src/main/java/net/fabricmc/fabric/api/gamerule/v1/rule/ValidateableRule.java

@@ -24,9 +24,10 @@
 public interface ValidateableRule {
 	/**
 	 * Validates if a rule can accept the input.
+	 * If valid, the input will be set as the rule's value.
 	 *
 	 * @param value the value to validate
-	 * @return true if the value can be accepted.
+	 * @return true if the value was accepted.
 	 */
 	boolean validate(String value);
 }

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/mixin/object/builder/AbstractBlockSettingsAccessor.java

@@ -27,6 +27,8 @@
 import net.minecraft.block.AbstractBlock;
 import net.minecraft.block.BlockState;
 import net.minecraft.block.MapColor;
+import net.minecraft.block.enums.Instrument;
+import net.minecraft.block.piston.PistonBehavior;
 import net.minecraft.entity.EntityType;
 import net.minecraft.resource.featuretoggle.FeatureSet;
 import net.minecraft.sound.BlockSoundGroup;
@@ -130,6 +132,27 @@ public interface AbstractBlockSettingsAccessor {
 	@Accessor
 	FeatureSet getRequiredFeatures();
 
+	@Accessor
+	boolean getBurnable();
+
+	@Accessor
+	boolean getLiquid();
+
+	@Accessor
+	boolean getForceNotSolid();
+
+	@Accessor
+	boolean getForceSolid();
+
+	@Accessor
+	PistonBehavior getPistonBehavior();
+
+	@Accessor
+	Instrument getInstrument();
+
+	@Accessor
+	boolean getReplaceable();
+
 	/* SETTERS */
 	@Accessor
 	void setCollidable(boolean collidable);
@@ -167,4 +190,19 @@ public interface AbstractBlockSettingsAccessor {
 
 	@Accessor
 	void setOffsetter(Optional<AbstractBlock.Offsetter> offsetter);
+
+	@Accessor
+	void setBurnable(boolean burnable);
+
+	@Accessor
+	void setLiquid(boolean liquid);
+
+	@Accessor
+	void setForceNotSolid(boolean forceNotSolid);
+
+	@Accessor
+	void setForceSolid(boolean forceSolid);
+
+	@Accessor
+	void setReplaceable(boolean replaceable);
 }

fabric-resource-conditions-api-v1/src/main/java/net/fabricmc/fabric/mixin/resource/conditions/LootManagerMixin.java

@@ -17,17 +17,27 @@
 
 package net.fabricmc.fabric.mixin.resource.conditions;
 
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
 
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Unique;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
+import org.spongepowered.asm.mixin.injection.callback.LocalCapture;
 
 import net.minecraft.loot.LootDataType;
 import net.minecraft.loot.LootManager;
+import net.minecraft.registry.DynamicRegistryManager;
+import net.minecraft.resource.ResourceManager;
 import net.minecraft.util.Identifier;
 
 import net.fabricmc.fabric.api.resource.conditions.v1.ResourceConditions;
@@ -38,6 +48,22 @@
  */
 @Mixin(LootManager.class)
 public class LootManagerMixin {
+	// Keep track of the DynamicRegistryManager instance by assgining it to the map that is passed to the async runnable.
+	@Unique
+	private static final Map<Object, DynamicRegistryManager.Immutable> dynamicRegistryManagerMap = Collections.synchronizedMap(new IdentityHashMap<>());
+
+	@Inject(method = "load", at = @At(value = "INVOKE", target = "Ljava/util/concurrent/CompletableFuture;runAsync(Ljava/lang/Runnable;Ljava/util/concurrent/Executor;)Ljava/util/concurrent/CompletableFuture;"), locals = LocalCapture.CAPTURE_FAILHARD)
+	private static void load(LootDataType type, ResourceManager resourceManager, Executor executor, Map<LootDataType<?>, Map<Identifier, ?>> results, CallbackInfoReturnable<CompletableFuture<?>> cir, Map map) {
+		dynamicRegistryManagerMap.put(map, ResourceConditionsImpl.CURRENT_REGISTRIES.get());
+	}
+
+	// runAsync Runnable in load method
+	@Inject(method = "method_51189", at = @At("HEAD"))
+	private static void runAsync(ResourceManager resourceManager, LootDataType lootDataType, Map map, CallbackInfo ci) {
+		assert ResourceConditionsImpl.CURRENT_REGISTRIES.get() == null;
+		ResourceConditionsImpl.CURRENT_REGISTRIES.set(Objects.requireNonNull(dynamicRegistryManagerMap.remove(map)));
+	}
+
 	// forEach in load method
 	@Inject(method = "method_51195", at = @At("HEAD"), cancellable = true)
 	private static void applyResourceConditions(LootDataType lootDataType, Map map, Identifier id, JsonElement json, CallbackInfo ci) {
@@ -58,4 +84,10 @@ private static void applyResourceConditions(LootDataType lootDataType, Map map,
 			}
 		}
 	}
+
+	// runAsync Runnable in load method
+	@Inject(method = "method_51189", at = @At("RETURN"))
+	private static void runAsyncEnd(ResourceManager resourceManager, LootDataType lootDataType, Map map, CallbackInfo ci) {
+		ResourceConditionsImpl.CURRENT_REGISTRIES.remove();
+	}
 }

fabric-resource-conditions-api-v1/src/main/resources/quilted_fabric_resource_conditions_api_v1.mixins.json

@@ -9,5 +9,8 @@
     "LootManagerMixin",
     "SinglePreparationResourceReloaderMixin",
     "TagManagerLoaderMixin"
-  ]
+  ],
+  "injectors": {
+    "defaultRequire": 1
+  }
 }

fabric-resource-conditions-api-v1/src/testmod/java/net/fabricmc/fabric/test/resource/conditions/ConditionalResourcesTest.java

@@ -88,4 +88,19 @@ public void conditionalPredicates(TestContext context) {
 
 		context.complete();
 	}
+
+	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
+	public void conditionalLootTables(TestContext context) {
+		LootManager manager = context.getWorld().getServer().getLootManager();
+
+		if (manager.getElementOptional(LootDataType.LOOT_TABLES, id("blocks/loaded")).isEmpty()) {
+			throw new AssertionError("loaded loot table should have been loaded.");
+		}
+
+		if (manager.getElementOptional(LootDataType.LOOT_TABLES, id("blocks/not_loaded")).isPresent()) {
+			throw new AssertionError("not_loaded loot table should not have been loaded.");
+		}
+
+		context.complete();
+	}
 }

fabric-resource-conditions-api-v1/src/testmod/resources/data/fabric-resource-conditions-api-v1-testmod/loot_tables/blocks/loaded.json

@@ -0,0 +1,29 @@
+{
+  "type": "minecraft:block",
+  "pools": [
+    {
+      "bonus_rolls": 0.0,
+      "conditions": [
+        {
+          "condition": "minecraft:survives_explosion"
+        }
+      ],
+      "entries": [
+        {
+          "type": "minecraft:item",
+          "name": "minecraft:cobblestone"
+        }
+      ],
+      "rolls": 1.0
+    }
+  ],
+  "random_sequence": "minecraft:blocks/cobblestone",
+  "fabric:load_conditions": [
+    {
+      "condition": "fabric:registry_contains",
+      "values": [
+        "minecraft:cobblestone"
+      ]
+    }
+  ]
+}

fabric-resource-conditions-api-v1/src/testmod/resources/data/fabric-resource-conditions-api-v1-testmod/loot_tables/blocks/not_loaded.json

@@ -0,0 +1,29 @@
+{
+  "type": "minecraft:block",
+  "pools": [
+    {
+      "bonus_rolls": 0.0,
+      "conditions": [
+        {
+          "condition": "minecraft:survives_explosion"
+        }
+      ],
+      "entries": [
+        {
+          "type": "minecraft:item",
+          "name": "minecraft:cobblestone"
+        }
+      ],
+      "rolls": 1.0
+    }
+  ],
+  "random_sequence": "minecraft:blocks/cobblestone",
+  "fabric:load_conditions": [
+    {
+      "condition": "fabric:registry_contains",
+      "values": [
+        "minecraft:does_not_exist"
+      ]
+    }
+  ]
+}

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java

@@ -19,6 +19,7 @@
 
 import java.util.Iterator;
 
+import com.google.common.collect.Iterators;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Nullable;
 
@@ -153,7 +154,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<StorageView<T>> nonEmptyIterator() {
-		return TransferApiImpl.filterEmptyViews(iterator());
+		return Iterators.filter(iterator(), view -> view.getAmount() > 0 && !view.isResourceBlank());
 	}
 
 	/**

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java

@@ -95,44 +95,6 @@ public T next() {
 		};
 	}
 
-	public static <T> Iterator<StorageView<T>> filterEmptyViews(Iterator<StorageView<T>> iterator) {
-		return new Iterator<>() {
-			StorageView<T> 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<T> next() {
-				if (!hasNext()) {
-					throw new NoSuchElementException();
-				}
-
-				StorageView<T> ret = next;
-				findNext();
-				return ret;
-			}
-		};
-	}
-
 	public static <T> List<SingleSlotStorage<T>> makeListView(SlottedStorage<T> storage) {
 		return new AbstractList<>() {
 			@Override

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/fluid/CombinedProvidersImpl.java

@@ -83,10 +83,14 @@ public Storage<FluidVariant> find(ItemStack itemStack, ContainerItemContext cont
 	}
 
 	public static Event<FluidStorage.CombinedItemApiProvider> getOrCreateItemEvent(Item item) {
-		// register here is thread-safe, so the query below will return a valid provider (possibly one registered before or from another thread).
-		FluidStorage.ITEM.registerForItems(new Provider(), item);
 		ItemApiLookup.ItemApiProvider<Storage<FluidVariant>, ContainerItemContext> existingProvider = FluidStorage.ITEM.getProvider(item);
 
+		if (existingProvider == null) {
+			FluidStorage.ITEM.registerForItems(new Provider(), item);
+			// The provider might not be new Provider() if a concurrent registration happened, re-query.
+			existingProvider = FluidStorage.ITEM.getProvider(item);
+		}
+
 		if (existingProvider instanceof Provider registeredProvider) {
 			return registeredProvider.event;
 		} else {

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java

@@ -20,18 +20,23 @@
 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;
 
 public class BaseStorageTests {
 	public static void run() {
 		testFilteringStorage();
+		testNonEmptyIteratorWithModifiedView();
 	}
 
 	private static void testFilteringStorage() {
@@ -93,4 +98,24 @@ protected boolean canInsert(FluidVariant resource) {
 
 		assertEquals(BUCKET, StorageUtil.simulateExtract(storage, lava, BUCKET, null));
 	}
+
+	/**
+	 * Regression test for <a href="https://github.com/FabricMC/fabric/issues/3414">
+	 * {@code nonEmptyIterator} not handling views that become empty during iteration correctly</a>.
+	 */
+	private static void testNonEmptyIteratorWithModifiedView() {
+		SingleVariantStorage<FluidVariant> storage = SingleFluidStorage.withFixedCapacity(BUCKET, () -> { });
+		storage.variant = FluidVariant.of(Fluids.WATER);
+
+		Iterator<StorageView<FluidVariant>> 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());
+	}
 }