Fabric Resource Conditions API

[Technici4n]

Closes #1644.

Example recipe

{
  "type": "minecraft:crafting_shapeless",
  "ingredients": [
    {
      "item": "minecraft:stick"
    }
  ],
  "result": {
    "item": "minecraft:diamond"
  },
  "fabric:conditions": [
    {
      "condition": "fabric:not",
      "value": {
        "condition": "fabric:all_mods_loaded",
        "values": [
          "a"
        ]
      }
    }
  ]
}

Example condition implementation, with the helper to generate json entries too:

// ID will be used multiple times
	private static final Identifier MODS_LOADED = new Identifier("fabric:mods_loaded");
// Only API we need is the json provider
	public static ConditionJsonProvider modsLoaded(String... modIds) {
		return new ConditionJsonProvider() {
			@Override
			public void serialize(JsonObject object) {
				JsonArray array = new JsonArray();
				for (var modId : modIds) {
					array.add(modId);
				}
				object.add("values", array);
			}

			@Override
			public Identifier getConditionIdentifier() {
				return MODS_LOADED;
			}
		};
	}
// Register the condition
	static {
		ResourceConditions.register(MODS_LOADED, object -> {
			JsonArray array = JsonHelper.getArray(object, "values");

			for (JsonElement element : array) {
				if (element.isJsonPrimitive()) {
					if (!FabricLoader.getInstance().isModLoaded(element.getAsString())) {
						return false;
					}
				} else {
					throw new JsonParseException("Invalid mod id entry: " + element);
				}
			}

			return true;
		});
	}

Please let me know what you think of the approach, and which additional conditions should be provided.

TODO:

• Recipe conditions.
• Integration in the datagen API.
• Validating testmod.
• Document the conditions we provide by default.
• Document where we check for conditions.

[haykam821]

Conditions should be registered using identifiers instead of strings.

[warjort]

Conditions should be registered using identifiers instead of strings.

That would make the real name of the builtin conditions something like minecraft:not
i.e. new Identifier("not") from "not": {}

BTW: isn't {} the same as "{"always": ""} , i.e. "always" is redundant?

[warjort]

i.e. "always" is redundant?

Actually is does have a use under a different name

 "fabric:conditions": {
  "comment": "some memo to describe the purpose of a complicated condition"
}

[warjort]

I don't know how useful it would be, but you can also add some syntatic sugar by supporting things like:

"not": false instead of "not": {"never": ""}

"ex falso quodlibet" :-)

[warjort]

            Predicate<JsonElement> condition = JsonResourceConditions.get(entry.getKey());

            if (condition == null) {
                throw new IllegalArgumentException("Encountered unknown recipe condition " + entry.getKey());
            } else if (condition.test(entry.getValue())) {
                return true;
            }

Shouldn't this error handling be inside the framework?
Having every implementation do this if they support thirdparty conditions seems error prone.

Something like:

boolean JsonResourceConditions::test(String id, JsonElement value) {
    // the above code
}

I am also thinking of possible future enhancement if additional cross cutting logic needs to applied when a condition is tested.
e.g. adding externally configured fallback conditions something like:

config/fabric_condition_aliases.json
{
  [
  "original":  {"mod_loaded": "foobar"}, 
  "alias": {"mod_loaded": "foobar_forked"}
  ]
)

That's just an example, not a serious suggestion. :-)

[Technici4n]

These are good suggestions (besides the last one :-P), but I think this API is pending a datagen API, as we will have to integrate it. Perhaps a more complex approach where serializers have to be registered could be useful for datagen, but for that we need a datagen API (and thus global access wideners... hopefully we will have those in a few weeks :)).

[kanpov]

What's the status on this PR?

[Technici4n]

Waiting for datagen API, which itself is waiting for #1802.

[AlphaMode]

@Technici4n Do you plan on working on this soon now that #1824 and #1802 are merged?

[kanpov]

@Technici4n Do you plan on working on this soon now that #1824 and #1802 are merged?

Welp, both of these PRs have already been merged, so...

[Technici4n]

Yes, see #api on discord for the moment as there are some early design decisions that need to be made.

[Technici4n]

Updated the PR with a new approach, going to continue this over the coming days.

[shartte]

Maybe this? I am not so sure about this...

Suggested change

	public static boolean objectMatchesConditions(JsonObject object) {
	public static boolean resourceMatchesConditions(JsonObject object) {

[Technici4n]

The idea is that these conditions can also be used for nested objects, not necessarily the resource itself. For example if you only want some outputs to be produced if a mod is loaded.

[Juuxel]

I still believe that a file/directory-level (instead of file content-based) approach would be simpler and easier to use. For one, it allows to exclude entire directories by default without duplicating the same fabric:load_conditions block to each JSON file. This works for mods that organise their mod compat files by the supported mod (data/adorn/loot_tables/blocks/terrestria etc) It also works automatically for each and every data type.

In any case, here's a review of what you have now.

[Juuxel]

This feels a bit cursed, but I can't think of any better solutions either

[liach]

I recall shedaniel's draft had metadata files defined for any resource file (like .mcmeta files used by vanilla, also JSON format), so those conditions work for any resource such as command functions than json-only. In addition, your patch can be easily bypassed if someone isn't using json data loader; I don't think tags use the json data loader.

[Juuxel]

I don't think tags use the json data loader

@liach Note that tags don't need it since the values can be marked as optional, in which case they're ignored if not present.

[Technici4n]

Message I wrote on discord:

So, regarding recipe conditions. There are basically two options:
1) extend some json formats, for example we'd extend the recipe json format with a fabric:conditions: {...}.
2) work at the resourcepack level directly, to allow blacklisting entire directories.
APPROACH 1
+ easy to implement
+ guaranteed not to slow down the resource manager
+ can implement conditions that depend on previous resource reload listeners. For example, you can have a tag_exists_with_at_least_one_entry recipe condition, which is very useful
+ not limited to root json objects; can also be used for sub-objects, e.g. to conditionally remove a loot pool from a loot table
- you need to manually add a conditions entry to each recipe (LARGELY if not entirely mitigated by datagen imo)
- we'll have to patch multiple json formats VS just patching the resource manager
APPROACH 2
+ much easier to use for simple compat recipes if you don't have datagen. Just place a .fabricmeta in the relevant directory and the conditions will be applied to all files in the directory
+ only requires resource manager patches VS having to patch multiple json formats
- much harder to implement
- might slow down the resource manager
- conditions can't depend on previous reload listeners
For better conditions, and because it's a lot easier, I suggest going for approach 1

TL;DR 1 is good enough, guaranteed not to be slower and much easier to implement.

[liach]

Shouldn't we try to merge the conditions if there are already existing ones?

[Technici4n]

Datagen already merges multiple ConditionJsonProvider[] together.

[liach]

It'd be better if we handle the null and the 0-length array case the same

[Technici4n]

The 0-length case is usually a programmer error, while the null case is conveniently used to skip unspecified entries in the datagen.

[liach]

The 0-length case is usually a programmer error, while the null case is conveniently used to skip unspecified entries in the datagen.

Then why not just ask the modders to not call this method if they have nothing to write to the conditions after all

[Technici4n]

Well I guess modders could make use of it in their own datagen if they need their own conditions

[modmuss50]

Going to merge and release now. Sorry for the delay ive been busy IRL 👍

[LemmaEOF]

https://github.com/cottonmc/libcd

[MJRamon]

How can we use it to set a potion as a recipe ingredient?