From 2332ecf984babeaca5dd98432bea33b00c3aec16 Mon Sep 17 00:00:00 2001 From: LexManos Date: Sun, 8 Oct 2023 12:14:21 -0700 Subject: [PATCH] Fix ConditionalCodec having an inverted test. Closes #9742 Remove clean project on CI. Bump FG version to fix transitive issue on module path. --- build.gradle | 104 ++++-------------- .../forge/tasks/FixPatchImports.groovy | 40 ------- build_clean.gradle | 51 +++++++++ mdk/build.gradle | 2 +- settings.gradle | 8 +- .../loot_tables/blocks/test.json | 34 ++++++ .../crafting/conditions/ConditionCodec.java | 4 +- .../gameplay/loot/ConditionalLootPools.java | 100 +++++++++++++++++ 8 files changed, 215 insertions(+), 128 deletions(-) delete mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/FixPatchImports.groovy create mode 100644 build_clean.gradle create mode 100644 src/generated_test/resources/conditional_loot_test/data/conditional_loot_test/loot_tables/blocks/test.json create mode 100644 src/test/java/net/minecraftforge/debug/gameplay/loot/ConditionalLootPools.java diff --git a/build.gradle b/build.gradle index 4d395fc4be..4377abd101 100644 --- a/build.gradle +++ b/build.gradle @@ -4,7 +4,6 @@ import net.minecraftforge.forge.tasks.checks.CheckExcs import net.minecraftforge.forge.tasks.checks.CheckPatches import net.minecraftforge.forge.tasks.checks.CheckSAS import net.minecraftforge.forge.tasks.checks.CheckTask -import net.minecraftforge.forge.tasks.FixPatchImports import java.nio.file.Files import java.util.Date @@ -28,8 +27,8 @@ plugins { id 'net.minecraftforge.gradleutils' version '2.+' id 'eclipse' id 'de.undercouch.download' version '5.4.0' - id 'net.minecraftforge.gradle.patcher' version '[6.0.13,6.2)' apply false - id 'net.minecraftforge.gradle.mcp' version '[6.0.13,6.2)' apply false + id 'net.minecraftforge.gradle.patcher' version '[6.0.14,6.2)' apply false + id 'net.minecraftforge.gradle.mcp' version '[6.0.14,6.2)' apply false } Util.init() //Init all our extension methods! @@ -115,6 +114,7 @@ tasks.register('downloadVersionManifest', Download) { quiet true } tasks.register('downloadJson', Download) { + dependsOn downloadVersionManifest inputs.file downloadVersionManifest.dest src { downloadVersionManifest.dest.json.versions.find{ it.id == MC_VERSION }.url } dest file("build/versions/${MC_VERSION}/version.json") @@ -123,6 +123,7 @@ tasks.register('downloadJson', Download) { quiet true } tasks.register('downloadClientRaw', Download) { + dependsOn downloadJson inputs.file downloadJson.dest src { downloadJson.dest.json.downloads.client.url } dest file("build/versions/${MC_VERSION}/client.jar") @@ -131,6 +132,7 @@ tasks.register('downloadClientRaw', Download) { quiet true } tasks.register('downloadServerRaw', Download) { + dependsOn downloadJson inputs.file downloadJson.dest src { downloadJson.dest.json.downloads.server.url } dest file("build/versions/${MC_VERSION}/server.jar") @@ -148,14 +150,6 @@ subprojects { version = VERSION //println(name + ' Version: ' + version) - ext { - isNonStable = { String version -> - def stableKeyword = ['RELEASE', 'FINAL', 'GA'].any { ver -> version.toUpperCase().contains(ver) } - def regex = /^[0-9,.v-]+(-r)?$/ - return !stableKeyword && !(version ==~ regex) - } - } - repositories { mavenLocal() mavenCentral() @@ -233,55 +227,6 @@ project(':mcp') { } } -project(':clean') { - evaluationDependsOn(':mcp') - apply plugin: 'net.minecraftforge.gradle.patcher' - tasks.withType(AbstractPublishToMaven.class).forEach{tsk -> tsk.enabled = false } // We don't want to publish anything - - dependencies { - implementation 'net.minecraftforge:forgespi:' + SPI_VERSION - } - - patcher { - parent = project(':mcp') - mcVersion = MC_VERSION - patchedSrc = file('src/main/java') - - mappings channel: MAPPING_CHANNEL, version: MAPPING_VERSION - - runs { - clean_client { - client true - taskName 'clean_client' - ideaModule "${rootProject.name}.${project.name}.main" - - main 'net.minecraft.client.main.Main' - workingDirectory project.file('run/client') - - args '--gameDir', '.' - args '--version', MC_VERSION - args '--assetsDir', downloadAssets.output - args '--assetIndex', '{asset_index}' - args '--accessToken', '0' - } - - clean_server { - client false - taskName 'clean_server' - ideaModule "${rootProject.name}.${project.name}.main" - - main 'net.minecraft.server.Main' - workingDirectory project.file('run/server') - } - } - } - eclipse.classpath.file.whenMerged { - // Disable optional warnings on the minecraft decompiled source code. It just muddies the warning window and hides warnings in our own codebase - def src = entries.find { it.path == 'src/main/java' } - src.entryAttributes['ignore_optional_problems'] = 'true' - } -} - def sharedDeps = { moduleonly("cpw.mods:securejarhandler:${SECUREJARHANDLER_VERSION}") moduleonly("org.ow2.asm:asm:${ASM_VERSION}") @@ -291,9 +236,9 @@ def sharedDeps = { moduleonly("org.ow2.asm:asm-analysis:${ASM_VERSION}") moduleonly("cpw.mods:bootstraplauncher:${BOOTSTRAPLAUNCHER_VERSION}") moduleonly("net.minecraftforge:JarJarFileSystems:${JARJAR_VERSION}") - moduleonly("com.google.jimfs:jimfs:${JIMFS_VERSION}") { transitive = false } - moduleonly("com.google.guava:guava:${GUAVA_VERSION}") { transitive = false } // Need by jimfs - moduleonly("com.google.guava:failureaccess:1.0.1"){ transitive = false} // Needed by guava + moduleonly("com.google.jimfs:jimfs:${JIMFS_VERSION}") + moduleonly("com.google.guava:guava:${GUAVA_VERSION}") // Need by jimfs + moduleonly("com.google.guava:failureaccess:1.0.1") // Needed by guava installer "cpw.mods:securejarhandler:${SECUREJARHANDLER_VERSION}" installer "org.ow2.asm:asm:${ASM_VERSION}" @@ -449,7 +394,7 @@ def sharedFmlonlyForge = { Project prj -> } } - def MODULE_PATH = Util.getArtifacts(prj, prj.configurations.moduleonly, false).values().collect{"libraries/${it.downloads.artifact.path}"} + def MODULE_PATH = Util.getArtifacts(prj, prj.configurations.moduleonly, false).values().collect{"libraries/${it.downloads.artifact.path}"}.sort() def tokens = [ TASK: 'forge_server', MAVEN_PATH: prj.ext.MAVEN_PATH.toString(), @@ -457,7 +402,7 @@ def sharedFmlonlyForge = { Project prj -> MC_VERSION: MC_VERSION, MCP_VERSION: MCP_VERSION, FORGE_GROUP: prj.group, - IGNORE_LIST: Util.getArtifacts(prj, prj.configurations.moduleonly, false).values().collect{it.downloads.artifact.path.rsplit('/', 1)[1]}.join(','), + IGNORE_LIST: Util.getArtifacts(prj, prj.configurations.moduleonly, false).values().collect{it.downloads.artifact.path.rsplit('/', 1)[1]}.sort().join(','), MODULES: 'ALL-MODULE-PATH' ] @@ -501,7 +446,7 @@ def sharedFmlonlyForge = { Project prj -> } project(':forge') { - evaluationDependsOn(':clean') + evaluationDependsOn(':mcp') apply plugin: 'java-library' apply plugin: 'net.minecraftforge.gradle.patcher' apply plugin: 'org.cadixdev.licenser' @@ -571,11 +516,9 @@ project(':forge') { } configurations { - installer { - // Don't pull all libraries, if we're missing something, add it to the installer list so the installer knows to download it. - transitive = false - } - moduleonly + // Don't pull all libraries, if we're missing something, add it to the installer list so the installer knows to download it. + installer { transitive = false } + moduleonly { transitive = false } api.extendsFrom(installer) } @@ -596,7 +539,9 @@ project(':forge') { patcher { excs.from file("$rootDir/src/main/resources/forge.exc") - parent = project(':clean') + parent = project(':mcp') + mcVersion = MC_VERSION + mappings channel: MAPPING_CHANNEL, version: MAPPING_VERSION patches = file("$rootDir/patches/minecraft") patchedSrc = file('src/main/java') srgPatches = false @@ -904,11 +849,6 @@ project(':forge') { ] } - tasks.register('fixPatchImports', FixPatchImports) { - clean.set(rootProject.file('projects/clean/src/main/java')) - patched.set(rootProject.file('projects/forge/src/main/java')) - } - genPatches { finalizedBy checkAndFixPatches autoHeader true @@ -1420,19 +1360,17 @@ project(':forge') { publish.dependsOn(':forge:checkJarCompatibility') } -//evaluationDependsOnChildren() -task setup() { //These must be strings so that we can do lazy resolution. Else we need evaluationDependsOnChildren above - dependsOn ':clean:extractMapped' +tasks.register('setup') { dependsOn ':forge:extractMapped' + if (findProject(':clean')) + dependsOn ':clean:extractMapped' } -task doChecks { +tasks.register('doChecks') { dependsOn ':forge:checkJarCompatibility' dependsOn ':forge:publish' } -project.gradle.startParameter.excludedTaskNames.add(':clean:genPatches') - if (System.env.TEAMCITY_VERSION) { //Only setup the CI environment if and only if the environment variables are set. tasks.configureTeamCity { diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/FixPatchImports.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/FixPatchImports.groovy deleted file mode 100644 index ed65bdd9aa..0000000000 --- a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/FixPatchImports.groovy +++ /dev/null @@ -1,40 +0,0 @@ -package net.minecraftforge.forge.tasks - -import org.gradle.api.DefaultTask -import org.gradle.api.file.DirectoryProperty -import org.gradle.api.tasks.InputDirectory -import org.gradle.api.tasks.TaskAction - -import java.nio.file.Files - -abstract class FixPatchImports extends DefaultTask { - - @InputDirectory abstract DirectoryProperty getClean() - @InputDirectory abstract DirectoryProperty getPatched() - - @TaskAction - void run() { - final patchedPath = getPatched().get().asFile.toPath() - final cleanPath = getClean().get().asFile.toPath() - - try (final var stream = Files.find(patchedPath, Integer.MAX_VALUE, (path, attr) -> path.toString().endsWith('.java'))) { - final itr = stream.iterator() - while (itr.hasNext()) { - final next = itr.next() - final clean = cleanPath.resolve(patchedPath.relativize(next).toString()) - - final patchedLines = Files.readAllLines(next) - final cleanLines = Files.readAllLines(clean) - - final patchedImport = patchedLines.findLastIndexOf(2) { it.startsWith('import ') } - final cleanImport = cleanLines.findLastIndexOf(2) { it.startsWith('import ') } - - if (cleanImport !== patchedImport) { - patchedLines.removeIf { it.startsWith('import ') } - patchedLines.addAll(2, cleanLines.subList(2, cleanImport + 1)) - Files.write(next, patchedLines) - } - } - } - } -} diff --git a/build_clean.gradle b/build_clean.gradle new file mode 100644 index 0000000000..d0472d28b8 --- /dev/null +++ b/build_clean.gradle @@ -0,0 +1,51 @@ +plugins { + id 'org.cadixdev.licenser' + id 'java-library' + id 'eclipse' + id 'net.minecraftforge.gradle.patcher' version '[6.0.14,6.2)' +} + +evaluationDependsOn(':mcp') + +dependencies { + implementation 'net.minecraftforge:forgespi:' + SPI_VERSION +} + +patcher { + parent = project(':mcp') + mcVersion = MC_VERSION + patchedSrc = file('src/main/java') + + mappings channel: MAPPING_CHANNEL, version: MAPPING_VERSION + + runs { + clean_client { + client true + taskName 'clean_client' + ideaModule "${rootProject.name}.${project.name}.main" + + main 'net.minecraft.client.main.Main' + workingDirectory project.file('run/client') + + args '--gameDir', '.' + args '--version', MC_VERSION + args '--assetsDir', downloadAssets.output + args '--assetIndex', '{asset_index}' + args '--accessToken', '0' + } + + clean_server { + client false + taskName 'clean_server' + ideaModule "${rootProject.name}.${project.name}.main" + + main 'net.minecraft.server.Main' + workingDirectory project.file('run/server') + } + } +} +eclipse.classpath.file.whenMerged { + // Disable optional warnings on the minecraft decompiled source code. It just muddies the warning window and hides warnings in our own codebase + def src = entries.find { it.path == 'src/main/java' } + src.entryAttributes['ignore_optional_problems'] = 'true' +} \ No newline at end of file diff --git a/mdk/build.gradle b/mdk/build.gradle index 5fa3a320de..adba699aed 100644 --- a/mdk/build.gradle +++ b/mdk/build.gradle @@ -2,7 +2,7 @@ plugins { id 'eclipse' id 'idea' id 'maven-publish' - id 'net.minecraftforge.gradle' version '[6.0,6.2)' + id 'net.minecraftforge.gradle' version '[6.0.14,6.2)' } version = mod_version diff --git a/settings.gradle b/settings.gradle index 8993137459..236b409b21 100644 --- a/settings.gradle +++ b/settings.gradle @@ -30,9 +30,13 @@ include 'lowcodelanguage' include 'fmlearlydisplay' include ':mcp' -include ':clean' include ':forge' project(":mcp").projectDir = file("projects/mcp") -project(":clean").projectDir = file("projects/clean") project(":forge").projectDir = file("projects/forge") + +if (!System.env.TEAMCITY_VERSION) { + include ':clean' + project(':clean').projectDir = file('projects/clean') + project(':clean').buildFileName = '../../build_clean.gradle' +} diff --git a/src/generated_test/resources/conditional_loot_test/data/conditional_loot_test/loot_tables/blocks/test.json b/src/generated_test/resources/conditional_loot_test/data/conditional_loot_test/loot_tables/blocks/test.json new file mode 100644 index 0000000000..713bd46f9a --- /dev/null +++ b/src/generated_test/resources/conditional_loot_test/data/conditional_loot_test/loot_tables/blocks/test.json @@ -0,0 +1,34 @@ +{ + "type": "minecraft:block", + "pools": [ + { + "bonus_rolls": 0.0, + "entries": [ + { + "type": "minecraft:item", + "name": "minecraft:diamond_block" + } + ], + "forge:condition": { + "type": "forge:false", + "value": {} + }, + "rolls": 1.0 + }, + { + "bonus_rolls": 0.0, + "entries": [ + { + "type": "minecraft:item", + "name": "minecraft:golden_apple" + } + ], + "forge:condition": { + "type": "forge:true", + "value": {} + }, + "rolls": 1.0 + } + ], + "random_sequence": "conditional_loot_test:blocks/test" +} \ No newline at end of file diff --git a/src/main/java/net/minecraftforge/common/crafting/conditions/ConditionCodec.java b/src/main/java/net/minecraftforge/common/crafting/conditions/ConditionCodec.java index c766d5d8ec..60284a6562 100644 --- a/src/main/java/net/minecraftforge/common/crafting/conditions/ConditionCodec.java +++ b/src/main/java/net/minecraftforge/common/crafting/conditions/ConditionCodec.java @@ -62,8 +62,8 @@ public class ConditionCodec { return DataResult.error(() -> conditionDecoded.error().get().message()); var condition = conditionDecoded.result().get(); - if (condition.test(getContext(ops))) - return DataResult.success(Pair.of(Optional.empty(), null)); + if (!condition.test(getContext(ops))) + return DataResult.success(Pair.of(Optional.empty(), ops.empty())); return normal.decode(ops, input).map(p -> p.mapFirst(Optional::of)); } diff --git a/src/test/java/net/minecraftforge/debug/gameplay/loot/ConditionalLootPools.java b/src/test/java/net/minecraftforge/debug/gameplay/loot/ConditionalLootPools.java new file mode 100644 index 0000000000..4d2e18ab8c --- /dev/null +++ b/src/test/java/net/minecraftforge/debug/gameplay/loot/ConditionalLootPools.java @@ -0,0 +1,100 @@ +/* + * Copyright (c) Forge Development LLC and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.minecraftforge.debug.gameplay.loot; + +import net.minecraft.core.BlockPos; +import net.minecraft.data.PackOutput; +import net.minecraft.data.loot.BlockLootSubProvider; +import net.minecraft.data.loot.LootTableProvider; +import net.minecraft.gametest.framework.GameTest; +import net.minecraft.gametest.framework.GameTestHelper; +import net.minecraft.world.flag.FeatureFlags; +import net.minecraft.world.item.Items; +import net.minecraft.world.level.block.Block; +import net.minecraft.world.level.block.state.BlockBehaviour; +import net.minecraft.world.level.storage.loot.LootPool; +import net.minecraft.world.level.storage.loot.LootTable; +import net.minecraft.world.level.storage.loot.entries.LootItem; +import net.minecraft.world.level.storage.loot.parameters.LootContextParamSets; +import net.minecraft.world.level.storage.loot.providers.number.ConstantValue; +import net.minecraftforge.eventbus.api.SubscribeEvent; +import net.minecraftforge.test.BaseTestMod; +import net.minecraftforge.fml.common.Mod; +import net.minecraftforge.common.crafting.conditions.IConditionBuilder; +import net.minecraftforge.data.event.GatherDataEvent; +import net.minecraftforge.gametest.GameTestHolder; +import net.minecraftforge.registries.DeferredRegister; +import net.minecraftforge.registries.ForgeRegistries; +import net.minecraftforge.registries.RegistryObject; + +import java.util.List; +import java.util.Set; + +@Mod(ConditionalLootPools.MODID) +@GameTestHolder("forge." + ConditionalLootPools.MODID) +public class ConditionalLootPools extends BaseTestMod { + public static final String MODID = "conditional_loot_test"; + + private static final DeferredRegister BLOCKS = DeferredRegister.create(ForgeRegistries.BLOCKS, MODID); + private static final RegistryObject TEST_BLOCK = BLOCKS.register("test", () -> new Block(BlockBehaviour.Properties.of())); + + @SubscribeEvent + public void runData(GatherDataEvent event) { + event.getGenerator().addProvider(event.includeServer(), new LootProvider(event.getGenerator().getPackOutput())); + } + + @GameTest(template = "forge:empty3x3x3") + public static void test_true_false(GameTestHelper helper) { + var center = new BlockPos(1, 1, 1); + helper.setBlock(center, TEST_BLOCK.get()); + helper.assertBlock(center, block -> block == TEST_BLOCK.get(), "Failed to set block"); + + var player = helper.makeMockServerPlayer(); + player.gameMode.destroyBlock(helper.absolutePos(center)); + + helper.assertItemEntityPresent(Items.GOLDEN_APPLE, center, 1.0); + + helper.succeed(); + } + + private static class LootProvider extends LootTableProvider { + public LootProvider(PackOutput out) { + super(out, Set.of(), List.of( + new LootTableProvider.SubProviderEntry(BlockLoot::new, LootContextParamSets.BLOCK) + )); + } + } + + private static class BlockLoot extends BlockLootSubProvider implements IConditionBuilder { + public BlockLoot() { + super(Set.of(), FeatureFlags.REGISTRY.allFlags()); + } + + @Override + protected Iterable getKnownBlocks() { + return List.of(TEST_BLOCK.get()); + } + + @Override + protected void generate() { + this.add(TEST_BLOCK.get(), LootTable.lootTable() + .withPool( + LootPool.lootPool() + .when(FALSE()) + .setRolls(ConstantValue.exactly(1.0F)) + .add(LootItem.lootTableItem(Items.DIAMOND_BLOCK)) + ) + .withPool( + LootPool.lootPool() + .when(TRUE()) + .setRolls(ConstantValue.exactly(1.0F)) + .add(LootItem.lootTableItem(Items.GOLDEN_APPLE)) + ) + ); + } + } + +} \ No newline at end of file