Fix recursion mess in PlayerAdvancement loading, by using an alternative based on a toposorted list of all advancements.

It seems to be fully equivalent in testing, but there may be corner cases, so there is a config to disable, though disabling
may break servers, especially those chunkloading fake players.

Signed-off-by: cpw <cpw+github@weeksfamily.ca>
This commit is contained in:
cpw 2020-04-19 21:35:49 -04:00
parent 699a5c3c18
commit 0512a47eba
No known key found for this signature in database
GPG key ID: 8EB3DF749553B1B7
4 changed files with 130 additions and 2 deletions

View file

@ -0,0 +1,10 @@
--- a/net/minecraft/advancements/AdvancementList.java
+++ b/net/minecraft/advancements/AdvancementList.java
@@ -96,6 +96,7 @@
}
}
+ net.minecraftforge.common.AdvancementLoadFix.buildSortedTrees(this.field_192093_c);
field_192091_a.info("Loaded {} advancements", (int)this.field_192092_b.size());
}

View file

@ -1,6 +1,14 @@
--- a/net/minecraft/advancements/PlayerAdvancements.java --- a/net/minecraft/advancements/PlayerAdvancements.java
+++ b/net/minecraft/advancements/PlayerAdvancements.java +++ b/net/minecraft/advancements/PlayerAdvancements.java
@@ -187,6 +187,8 @@ @@ -154,6 +154,7 @@
}
this.func_192748_e();
+ if (net.minecraftforge.common.ForgeConfig.SERVER.fixAdvancementLoading.get()) net.minecraftforge.common.AdvancementLoadFix.loadVisibility(this, this.field_192759_g, this.field_192760_h, this.field_192758_f, this.field_192761_i, this::func_192738_c); else
this.func_192752_d();
this.func_192751_c();
}
@@ -187,6 +188,8 @@
} }
public boolean func_192750_a(Advancement p_192750_1_, String p_192750_2_) { public boolean func_192750_a(Advancement p_192750_1_, String p_192750_2_) {
@ -9,7 +17,7 @@
boolean flag = false; boolean flag = false;
AdvancementProgress advancementprogress = this.func_192747_a(p_192750_1_); AdvancementProgress advancementprogress = this.func_192747_a(p_192750_1_);
boolean flag1 = advancementprogress.func_192105_a(); boolean flag1 = advancementprogress.func_192105_a();
@@ -199,6 +201,7 @@ @@ -199,6 +202,7 @@
if (p_192750_1_.func_192068_c() != null && p_192750_1_.func_192068_c().func_193220_i() && this.field_192762_j.field_70170_p.func_82736_K().func_223586_b(GameRules.field_223620_w)) { if (p_192750_1_.func_192068_c() != null && p_192750_1_.func_192068_c().func_193220_i() && this.field_192762_j.field_70170_p.func_82736_K().func_223586_b(GameRules.field_223620_w)) {
this.field_192756_d.func_184103_al().func_148539_a(new TranslationTextComponent("chat.type.advancement." + p_192750_1_.func_192068_c().func_192291_d().func_192307_a(), this.field_192762_j.func_145748_c_(), p_192750_1_.func_193123_j())); this.field_192756_d.func_184103_al().func_148539_a(new TranslationTextComponent("chat.type.advancement." + p_192750_1_.func_192068_c().func_192291_d().func_192307_a(), this.field_192762_j.func_145748_c_(), p_192750_1_.func_193123_j()));
} }

View file

@ -0,0 +1,104 @@
/*
* Minecraft Forge
* Copyright (c) 2016-2019.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation version 2.1
* of the License.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
package net.minecraftforge.common;
import com.google.common.graph.Graph;
import com.google.common.graph.GraphBuilder;
import com.google.common.graph.MutableGraph;
import net.minecraft.advancements.Advancement;
import net.minecraft.advancements.AdvancementProgress;
import net.minecraft.advancements.PlayerAdvancements;
import net.minecraftforge.fml.loading.toposort.TopologicalSort;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
/**
* New implementation of {@link net.minecraft.advancements.PlayerAdvancements#ensureAllVisible()}
*/
public class AdvancementLoadFix {
private static final Logger LOGGER = LogManager.getLogger();
private static Map<Advancement, List<Advancement>> roots;
public static void loadVisibility(final PlayerAdvancements playerAdvancements, final Set<Advancement> visible, final Set<Advancement> visibilityChanged, final Map<Advancement, AdvancementProgress> progress, final Set<Advancement> progressChanged, final Predicate<Advancement> shouldBeVisible) {
LOGGER.info("Using new advancement loading for {}", playerAdvancements);
if (roots == null) throw new RuntimeException("Why did the advancements not load yet?!");
final Set<Advancement> set = new HashSet<>();
for(Map.Entry<Advancement, AdvancementProgress> entry : progress.entrySet()) {
if (entry.getValue().isDone()) {
set.add(entry.getKey());
progressChanged.add(entry.getKey());
}
}
roots.values().stream().flatMap(Collection::stream).filter(adv -> containsAncestor(adv, set)).forEach(adv->updateVisibility(adv, visible, visibilityChanged, progress, progressChanged, shouldBeVisible));
}
private static boolean containsAncestor(final Advancement adv, final Set<Advancement> set) {
return set.contains(adv) || (adv.getParent() != null && containsAncestor(adv.getParent(), set));
}
private static void updateVisibility(final Advancement adv, final Set<Advancement> visible, final Set<Advancement> visibilityChanged, final Map<Advancement, AdvancementProgress> progress, final Set<Advancement> progressChanged, Predicate<Advancement> shouldBeVisible) {
boolean flag = shouldBeVisible.test(adv);
boolean flag1 = visible.contains(adv);
if (flag && !flag1) {
visible.add(adv);
visibilityChanged.add(adv);
if (progress.containsKey(adv)) {
progressChanged.add(adv);
}
} else if (!flag && flag1) {
visible.remove(adv);
visibilityChanged.add(adv);
}
if (flag!=flag1 && adv.getParent()!=null)
updateVisibility(adv.getParent(), visible, visibilityChanged, progress, progressChanged, shouldBeVisible);
}
public static void buildSortedTrees(final Set<Advancement> roots) {
AdvancementLoadFix.roots = roots.stream()
.map(AdvancementLoadFix::buildGraph)
.map(g -> TopologicalSort.topologicalSort(g, Comparator.comparing(Advancement::getId)))
.collect(Collectors.toMap(lst -> lst.get(0), Function.identity()));
}
private static Graph<Advancement> buildGraph(final Advancement root) {
final MutableGraph<Advancement> tree = GraphBuilder.directed().build();
addEdgesAndChildren(root, tree);
return tree;
}
private static void addEdgesAndChildren(final Advancement root, final MutableGraph<Advancement> tree) {
tree.addNode(root);
for (Advancement adv: root.getChildren()) {
addEdgesAndChildren(adv, tree);
tree.putEdge(root, adv);
}
}
}

View file

@ -50,6 +50,8 @@ public class ForgeConfig
public final BooleanValue treatEmptyTagsAsAir; public final BooleanValue treatEmptyTagsAsAir;
public final BooleanValue fixAdvancementLoading;
Server(ForgeConfigSpec.Builder builder) { Server(ForgeConfigSpec.Builder builder) {
builder.comment("Server configuration settings") builder.comment("Server configuration settings")
.push("server"); .push("server");
@ -110,6 +112,10 @@ public class ForgeConfig
.translation("forge.configgui.treatEmptyTagsAsAir") .translation("forge.configgui.treatEmptyTagsAsAir")
.define("treatEmptyTagsAsAir", false); .define("treatEmptyTagsAsAir", false);
fixAdvancementLoading = builder
.comment("Fix advancement loading to use a proper topological sort. This may have visibility side-effects and can thus be turned off if needed for data-pack compatibility.")
.translation("forge.configgui.fixAdvancementLoading")
.define("fixAdvancementLoading", true);
builder.pop(); builder.pop();
} }
} }