Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.21] Resource Conditions in Pack Overlays #3872

Merged
merged 7 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion fabric-resource-conditions-api-v1/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ loom {
accessWidenerPath = file("src/main/resources/fabric-resource-conditions-api-v1.accesswidener")
}

testDependencies(project, [':fabric-gametest-api-v1'])
testDependencies(project, [
':fabric-gametest-api-v1',
':fabric-lifecycle-events-v1',
':fabric-resource-loader-v0'
])
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ public final class ResourceConditions {
*/
public static final String CONDITIONS_KEY = "fabric:load_conditions";

/**
* The JSON key for conditional overlays in pack.mcmeta files.
*/
public static final String OVERLAYS_KEY = "fabric:overlays";

private ResourceConditions() {
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright (c) 2016, 2017, 2018, 2019 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.fabricmc.fabric.impl.resource.conditions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this a public API class? Without this class pack.mcmeta datagen is impossible

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I see this was asked to be changed a month ago — in my defence, the PR kept getting closed and it was a bit hard to keep track of.

In any case I still think this class should be an API anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definally be able to datagenerate this, however I worry just moving this to API isnt the ideal solution.

  • Generally we dont make records/data classes part of the API, we would have an interface + static factory function, it provides a better API surface.
  • How easy is it to datagenerate the pack.mcmeta file right now? I dont think we do anything special for it, this does sound like a useful thing to have. Maybe its best leaving this as-is for now and making a nice API for generating the file?

Copy link
Contributor

@maityyy maityyy Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we dont make records/data classes part of the API

However, this class is very similar to (even copies) the vanilla PackOverlaysMetadata, which makes it easy to understand how to use it

How easy is it to datagenerate the pack.mcmeta file right now?

The only thing is that ResourceFilter requires AW for BlockEntry::<init>, but that's the only such case. I datagen pack.mcmeta to filter recipes & advancements
(yes, another TAW candidate)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, actually, the static factory function sounds ok, the main thing is to give a name similar to the vanilla class (yarn)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to merge this PR as-is, we can have a follow up PR making this better for datagen.


import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;

import com.mojang.serialization.Codec;
import com.mojang.serialization.DataResult;
import com.mojang.serialization.codecs.RecordCodecBuilder;

import net.minecraft.resource.metadata.ResourceMetadataSerializer;

import net.fabricmc.fabric.api.resource.conditions.v1.ResourceCondition;
import net.fabricmc.fabric.api.resource.conditions.v1.ResourceConditions;

public record OverlayConditionsMetadata(List<Entry> overlays) {
public static final Codec<OverlayConditionsMetadata> CODEC = Entry.CODEC.listOf().fieldOf("entries").xmap(OverlayConditionsMetadata::new, OverlayConditionsMetadata::overlays).codec();
public static final ResourceMetadataSerializer<OverlayConditionsMetadata> SERIALIZER = ResourceMetadataSerializer.fromCodec(ResourceConditions.OVERLAYS_KEY, CODEC);

public List<String> appliedOverlays() {
List<String> appliedOverlays = new ArrayList<>();

for (Entry entry : this.overlays()) {
if (entry.condition().test(null)) {
appliedOverlays.add(entry.directory());
}
}

return appliedOverlays;
}

public record Entry(String directory, ResourceCondition condition) {
public static final Codec<Entry> CODEC = RecordCodecBuilder.create(instance -> instance.group(
Codec.STRING.validate(Entry::validateDirectory).fieldOf("directory").forGetter(Entry::directory),
ResourceCondition.CODEC.fieldOf("condition").forGetter(Entry::condition)
).apply(instance, Entry::new));
private static final Pattern DIRECTORY_NAME_PATTERN = Pattern.compile("[-_a-zA-Z0-9.]+");

private static DataResult<String> validateDirectory(String directory) {
boolean valid = DIRECTORY_NAME_PATTERN.matcher(directory).matches();
return valid ? DataResult.success(directory) : DataResult.error(() -> "Directory name is invalid");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2016, 2017, 2018, 2019 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.fabricmc.fabric.mixin.resource.conditions;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

import com.llamalad7.mixinextras.sugar.Local;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.ModifyVariable;

import net.minecraft.resource.ResourcePack;
import net.minecraft.resource.ResourcePackProfile;

import net.fabricmc.fabric.impl.resource.conditions.OverlayConditionsMetadata;

@Mixin(ResourcePackProfile.class)
public class ResourcePackProfileMixin {
@ModifyVariable(method = "loadMetadata", at = @At("STORE"))
private static List<String> applyOverlayConditions(List<String> overlays, @Local ResourcePack resourcePack) throws IOException {
List<String> appliedOverlays = new ArrayList<>(overlays);
OverlayConditionsMetadata overlayMetadata = resourcePack.parseMetadata(OverlayConditionsMetadata.SERIALIZER);

if (overlayMetadata != null) {
appliedOverlays.addAll(overlayMetadata.appliedOverlays());
}

return List.copyOf(appliedOverlays);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"RecipeManagerMixin",
"RegistryLoaderMixin",
"ReloadableRegistriesMixin",
"ResourcePackProfileMixin",
"ServerAdvancementLoaderMixin",
"SinglePreparationResourceReloaderMixin",
"TagManagerLoaderMixin"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,19 @@ public void conditionalDynamicRegistry(TestContext context) {

context.complete();
}

@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
public void conditionalOverlays(TestContext context) {
ReloadableRegistries.Lookup registries = context.getWorld().getServer().getReloadableRegistries();

if (!registries.getRegistryManager().get(RegistryKeys.PREDICATE).containsId(id("do_overlay"))) {
throw new AssertionError("do_overlay predicate should have been overlayed.");
}

if (registries.getRegistryManager().get(RegistryKeys.PREDICATE).containsId(id("dont_overlay"))) {
throw new AssertionError("dont_overlay predicate should not have been overlayed.");
}

context.complete();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"condition": "minecraft:survives_explosion"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"condition": "minecraft:survives_explosion"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"pack": {
"pack_format": 48,
"description": ""
},
"fabric:overlays": {
"entries": [
{
"directory": "do_overlay",
"condition": {
"condition": "fabric:true"
}
},
{
"directory": "dont_overlay",
"condition": {
"condition": "fabric:not",
"value": {
"condition": "fabric:true"
}
}
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

import net.minecraft.recipe.RecipeManager;
import net.minecraft.registry.RegistryWrapper;
import net.minecraft.resource.OverlayResourcePack;
import net.minecraft.resource.ResourcePack;
import net.minecraft.resource.ResourcePackInfo;
import net.minecraft.resource.ResourcePackPosition;
Expand Down Expand Up @@ -145,8 +146,19 @@ public ResourcePack open(ResourcePackInfo var1) {

@Override
public ResourcePack openWithOverlays(ResourcePackInfo var1, ResourcePackProfile.Metadata metadata) {
// Don't support overlays in builtin res packs.
return entry.getRight();
ModNioResourcePack pack = entry.getRight();

if (metadata.overlays().isEmpty()) {
return pack;
}

List<ResourcePack> overlays = new ArrayList<>(metadata.overlays().size());

for (String overlay : metadata.overlays()) {
overlays.add(pack.createOverlay(overlay));
}

return new OverlayResourcePack(pack, overlays);
}
}, resourceType, info2);
consumer.accept(profile);
Expand Down
Loading