Simplify FlagCondition (#1713)

neoforged · Nov 28, 2024 · 7730007 · 7730007
1 parent c0dbe21
commit 7730007
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 176 deletions.
diff --git a/src/main/java/net/neoforged/neoforge/common/NeoForgeMod.java b/src/main/java/net/neoforged/neoforge/common/NeoForgeMod.java
@@ -87,7 +87,7 @@
 import net.neoforged.neoforge.common.advancements.critereon.SnowBootsEntityPredicate;
 import net.neoforged.neoforge.common.conditions.AndCondition;
 import net.neoforged.neoforge.common.conditions.FalseCondition;
-import net.neoforged.neoforge.common.conditions.FlagCondition;
+import net.neoforged.neoforge.common.conditions.FeatureFlagsEnabledCondition;
 import net.neoforged.neoforge.common.conditions.ICondition;
 import net.neoforged.neoforge.common.conditions.ItemExistsCondition;
 import net.neoforged.neoforge.common.conditions.ModLoadedCondition;
@@ -386,7 +386,7 @@ public class NeoForgeMod {
     public static final DeferredHolder<MapCodec<? extends ICondition>, MapCodec<OrCondition>> OR_CONDITION = CONDITION_CODECS.register("or", () -> OrCondition.CODEC);
     public static final DeferredHolder<MapCodec<? extends ICondition>, MapCodec<TagEmptyCondition>> TAG_EMPTY_CONDITION = CONDITION_CODECS.register("tag_empty", () -> TagEmptyCondition.CODEC);
     public static final DeferredHolder<MapCodec<? extends ICondition>, MapCodec<TrueCondition>> TRUE_CONDITION = CONDITION_CODECS.register("true", () -> TrueCondition.CODEC);
-    public static final DeferredHolder<MapCodec<? extends ICondition>, MapCodec<FlagCondition>> FEATURE_FLAG_CONDITION = CONDITION_CODECS.register("feature_flags", () -> FlagCondition.CODEC);
+    public static final DeferredHolder<MapCodec<? extends ICondition>, MapCodec<FeatureFlagsEnabledCondition>> FEATURE_FLAGS_ENABLED_CONDITION = CONDITION_CODECS.register("feature_flags_enabled", () -> FeatureFlagsEnabledCondition.CODEC);
 
     private static final DeferredRegister<MapCodec<? extends EntitySubPredicate>> ENTITY_PREDICATE_CODECS = DeferredRegister.create(Registries.ENTITY_SUB_PREDICATE_TYPE, NeoForgeVersion.MOD_ID);
     public static final DeferredHolder<MapCodec<? extends EntitySubPredicate>, MapCodec<PiglinNeutralArmorEntityPredicate>> PIGLIN_NEUTRAL_ARMOR_PREDICATE = ENTITY_PREDICATE_CODECS.register("piglin_neutral_armor", () -> PiglinNeutralArmorEntityPredicate.CODEC);

diff --git a/src/main/java/net/neoforged/neoforge/common/conditions/FeatureFlagsEnabledCondition.java b/src/main/java/net/neoforged/neoforge/common/conditions/FeatureFlagsEnabledCondition.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) NeoForged and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.neoforged.neoforge.common.conditions;
+
+import com.mojang.serialization.MapCodec;
+import com.mojang.serialization.codecs.RecordCodecBuilder;
+import net.minecraft.world.flag.FeatureFlag;
+import net.minecraft.world.flag.FeatureFlagSet;
+import net.minecraft.world.flag.FeatureFlags;
+
+/**
+ * Condition checking that a set of {@link FeatureFlag feature flags} are enabled.
+ *
+ * @apiNote Mainly to be used when flagged content is not contained within the same feature pack which also enables said {@link FeatureFlag feature flags}.
+ */
+public record FeatureFlagsEnabledCondition(FeatureFlagSet flags) implements ICondition {
+    public static final MapCodec<FeatureFlagsEnabledCondition> CODEC = RecordCodecBuilder.mapCodec(instance -> instance.group(
+            FeatureFlags.CODEC.fieldOf("flags").forGetter(condition -> condition.flags)).apply(instance, FeatureFlagsEnabledCondition::new));
+
+    public FeatureFlagsEnabledCondition {
+        if (flags.isEmpty()) {
+            throw new IllegalArgumentException("FeatureFlagsEnabledCondition requires a non-empty feature flag set");
+        }
+    }
+
+    @Override
+    public boolean test(IContext context) {
+        return flags.isSubsetOf(context.enabledFeatures());
+    }
+
+    @Override
+    public MapCodec<? extends ICondition> codec() {
+        return CODEC;
+    }
+}
diff --git a/src/main/java/net/neoforged/neoforge/common/conditions/FlagCondition.java b/src/main/java/net/neoforged/neoforge/common/conditions/FlagCondition.java
diff --git a/src/main/java/net/neoforged/neoforge/common/conditions/IConditionBuilder.java b/src/main/java/net/neoforged/neoforge/common/conditions/IConditionBuilder.java
@@ -10,6 +10,7 @@
 import net.minecraft.world.flag.FeatureFlag;
 import net.minecraft.world.flag.FeatureFlagSet;
 import net.minecraft.world.item.Item;
+import org.apache.commons.lang3.ArrayUtils;
 
 public interface IConditionBuilder {
     default ICondition and(ICondition... values) {
@@ -44,27 +45,18 @@ default ICondition tagEmpty(TagKey<Item> tag) {
         return new TagEmptyCondition(tag.location());
     }
 
-    default ICondition isFeatureEnabled(FeatureFlagSet requiredFeatures) {
-        return FlagCondition.isEnabled(requiredFeatures);
+    default ICondition featureFlagsEnabled(FeatureFlagSet requiredFeatures) {
+        return new FeatureFlagsEnabledCondition(requiredFeatures);
     }
 
-    default ICondition isFeatureEnabled(FeatureFlag requiredFlag) {
-        return FlagCondition.isEnabled(requiredFlag);
-    }
-
-    default ICondition isFeatureEnabled(FeatureFlag requiredFlag, FeatureFlag... requiredFlags) {
-        return FlagCondition.isEnabled(requiredFlag, requiredFlags);
-    }
-
-    default ICondition isFeatureDisabled(FeatureFlagSet requiredFeatures) {
-        return FlagCondition.isDisabled(requiredFeatures);
-    }
-
-    default ICondition isFeatureDisabled(FeatureFlag requiredFlag) {
-        return FlagCondition.isDisabled(requiredFlag);
-    }
-
-    default ICondition isFeatureDisabled(FeatureFlag requiredFlag, FeatureFlag... requiredFlags) {
-        return FlagCondition.isDisabled(requiredFlag, requiredFlags);
+    default ICondition featureFlagsEnabled(FeatureFlag... requiredFlags) {
+        if (requiredFlags.length == 0) {
+            throw new IllegalArgumentException("FeatureFlagsEnabledCondition requires at least one feature flag.");
+        }
+        if (requiredFlags.length == 1) {
+            return new FeatureFlagsEnabledCondition(FeatureFlagSet.of(requiredFlags[0]));
+        } else {
+            return new FeatureFlagsEnabledCondition(FeatureFlagSet.of(requiredFlags[0], ArrayUtils.remove(requiredFlags, 0)));
+        }
     }
 }
diff --git a/...ources/data/neotests_test_flag_condition/advancement/recipes/misc/diamonds_from_dirt.json b/...ources/data/neotests_test_flag_condition/advancement/recipes/misc/diamonds_from_dirt.json
@@ -1,7 +1,7 @@
 {
   "neoforge:conditions": [
     {
-      "type": "neoforge:feature_flags",
+      "type": "neoforge:feature_flags_enabled",
       "flags": [
         "custom_feature_flags_pack_test:test_flag"
       ]

diff --git a/...ources/data/neotests_test_flag_condition/advancement/recipes/misc/dirt_from_diamonds.json b/...ources/data/neotests_test_flag_condition/advancement/recipes/misc/dirt_from_diamonds.json
diff --git a/.../src/generated/resources/data/neotests_test_flag_condition/recipe/diamonds_from_dirt.json b/.../src/generated/resources/data/neotests_test_flag_condition/recipe/diamonds_from_dirt.json
@@ -1,7 +1,7 @@
 {
   "neoforge:conditions": [
     {
-      "type": "neoforge:feature_flags",
+      "type": "neoforge:feature_flags_enabled",
       "flags": [
         "custom_feature_flags_pack_test:test_flag"
       ]

diff --git a/.../src/generated/resources/data/neotests_test_flag_condition/recipe/dirt_from_diamonds.json b/.../src/generated/resources/data/neotests_test_flag_condition/recipe/dirt_from_diamonds.json
diff --git a/tests/src/main/java/net/neoforged/neoforge/debug/data/CustomFeatureFlagsTests.java b/tests/src/main/java/net/neoforged/neoforge/debug/data/CustomFeatureFlagsTests.java
@@ -23,8 +23,7 @@
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.Items;
 import net.minecraft.world.level.Level;
-import net.neoforged.neoforge.common.Tags;
-import net.neoforged.neoforge.common.conditions.FlagCondition;
+import net.neoforged.neoforge.common.conditions.IConditionBuilder;
 import net.neoforged.neoforge.event.AddPackFindersEvent;
 import net.neoforged.neoforge.event.server.ServerStartedEvent;
 import net.neoforged.neoforge.registries.DeferredItem;
@@ -110,27 +109,25 @@ static void testFlagCondition(DynamicTest test, RegistrationHelper reg) {
 
         var modId = reg.modId();
         var enabledRecipeName = ResourceKey.create(Registries.RECIPE, ResourceLocation.fromNamespaceAndPath(modId, "diamonds_from_dirt"));
-        var disabledRecipeName = ResourceKey.create(Registries.RECIPE, ResourceLocation.fromNamespaceAndPath(modId, "dirt_from_diamonds"));
 
         reg.addProvider(event -> new RecipeProvider.Runner(event.getGenerator().getPackOutput(), event.getLookupProvider()) {
             @Override
             protected RecipeProvider createRecipeProvider(HolderLookup.Provider registries, RecipeOutput output) {
-                return new RecipeProvider(registries, output) {
+                class Provider extends RecipeProvider implements IConditionBuilder {
+                    protected Provider(HolderLookup.Provider p_360573_, RecipeOutput p_360872_) {
+                        super(p_360573_, p_360872_);
+                    }
+
                     @Override
                     protected void buildRecipes() {
                         // recipe available when above flag is enabled
                         shapeless(RecipeCategory.MISC, Items.DIAMOND)
                                 .requires(ItemTags.DIRT)
                                 .unlockedBy("has_dirt", has(ItemTags.DIRT))
-                                .save(output.withConditions(FlagCondition.isEnabled(flag)), enabledRecipeName);
-
-                        // recipe available when above flag is disabled
-                        shapeless(RecipeCategory.MISC, Items.DIRT)
-                                .requires(Tags.Items.GEMS_DIAMOND)
-                                .unlockedBy("has_diamond", has(Tags.Items.GEMS_DIAMOND))
-                                .save(output.withConditions(FlagCondition.isDisabled(flag)), disabledRecipeName);
+                                .save(output.withConditions(featureFlagsEnabled(flag)), enabledRecipeName);
                     }
-                };
+                }
+                return new Provider(registries, output);
             }
 
             @Override
@@ -144,22 +141,15 @@ public String getName() {
             var isFlagEnabled = server.getWorldData().enabledFeatures().contains(flag);
             var recipeMap = server.getRecipeManager().recipeMap();
             var hasEnabledRecipe = recipeMap.byKey(enabledRecipeName) != null;
-            var hasDisabledRecipe = recipeMap.byKey(disabledRecipeName) != null;
 
             if (isFlagEnabled) {
                 if (!hasEnabledRecipe) {
                     test.fail("Missing recipe '" + enabledRecipeName.location() + "', This should be enabled due to our flag '" + flagName + "' being enabled");
                 }
-                if (hasDisabledRecipe) {
-                    test.fail("Found recipe '" + disabledRecipeName.location() + "', This should be disabled due to our flag '" + flagName + "' being disabled");
-                }
             } else {
                 if (hasEnabledRecipe) {
                     test.fail("Found recipe '" + enabledRecipeName.location() + "', This should be disabled due to our flag '" + flagName + "' being enabled");
                 }
-                if (!hasDisabledRecipe) {
-                    test.fail("Missing recipe '" + disabledRecipeName.location() + "', This should be enabled due to our flag '" + flagName + "' being disabled");
-                }
             }
 
             test.pass();