Skip to content

Commit

Permalink
Start working on error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@OroArmor
OroArmor committed Aug 6, 2024
1 parent 1cae238 commit ada7ce0
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/main/java/org/quiltmc/loader/impl/plugin/QuiltPluginManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,8 +959,8 @@ private void appendModTable(Consumer<String> to) {
// - loader plugin
// - source path(s)
row.put(modColumn, mod.metadata().name());
row.put(id, mod.metadata().id());
row.put(version, mod.metadata().version());
row.put(id, mod.id());
row.put(version, mod.version());
row.put(plugin, mod.loader().pluginId());
StringBuilder flagStr = new StringBuilder();
flagStr.append(theQuiltPlugin.hasDepsChanged(mod) ? QuiltLoaderImpl.FLAG_DEPS_CHANGED : '.');
Expand Down
110 changes: 83 additions & 27 deletions src/main/java/org/quiltmc/loader/impl/plugin/SolverErrorHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void reportSolverError(Collection<Rule> rules) {

for (LoadOption load: rule.getNodesFrom())
graph.computeIfAbsent(options.get(load), option -> new ArrayList<>())
.add(new BreaksAll(breaks));
.add(new BreaksAll(breaks, (QuiltRuleBreakAll) rule));
} else if (rule instanceof QuiltRuleBreakOnly) {
Option breaks = rule.getNodesTo().stream()
.map(options::get)
Expand All @@ -149,7 +149,7 @@ void reportSolverError(Collection<Rule> rules) {

for (LoadOption load: rule.getNodesFrom())
graph.computeIfAbsent(options.get(load), option -> new ArrayList<>())
.add(new Breaks(breaks));
.add(new Breaks(breaks, ((QuiltRuleBreakOnly) rule)));
} else if (rule instanceof QuiltRuleDepAny) {
Set<Option> depends = rule.getNodesTo()
.stream()
Expand Down Expand Up @@ -210,9 +210,9 @@ void reportSolverError(Collection<Rule> rules) {
}
}

graph.remove(null);
printGraph(graph);
// System.out.println(graph);

graph.remove(null);
boolean modified = false;
do {
modified = false;
Expand All @@ -237,13 +237,44 @@ void reportSolverError(Collection<Rule> rules) {

printGraph(graph);

if (reportKnownSolverError(rootRules)) {
return;
}
reportNewError(graph);

// if (reportKnownSolverError(rootRules)) {
// return;
// }

addError(new UnhandledError(rules));
}

void reportNewError(Map<Option, Collection<Link>> graph) {
for (Option option: graph.keySet()) {
for (Link link: graph.get(option)) {
if (link instanceof Breaks) {
Breaks breaks = ((Breaks) link);
ModLoadOption from = (ModLoadOption) option.option;
Set<ModLoadOption> allBreakingOptions = new LinkedHashSet<>();
allBreakingOptions.addAll(breaks.rule.getConflictingOptions());
this.addError(new BreakageError(breaks.rule.publicDep, from, allBreakingOptions));
} else if (link instanceof BreaksAll) {
BreaksAll breaksAll = ((BreaksAll) link);
ModLoadOption from = (ModLoadOption) option.option;
Set<ModLoadOption> allBreakingOptions = new LinkedHashSet<>();
for (QuiltRuleBreakOnly only : breaksAll.rule.options) {
allBreakingOptions.addAll(only.getConflictingOptions());
}

for (ModDependency.Only only: breaksAll.rule.publicDep) {
ModDependencyIdentifier modOn = only.id();
VersionRange versionsOn = only.versionRange();
String reason = only.reason();
// TODO: BreakageAll?
// this.addError(new BreakageError(modOn, versionsOn, from, allBreakingOptions, reason));
}
}
}
}
}

static void printGraph(Map<Option, Collection<Link>> graph) {
System.out.println("digraph G {");
System.out.printf("\troot[style=invis];\n");
Expand Down Expand Up @@ -312,10 +343,12 @@ default void dotGraphEdge(Option from) {
};

static class Breaks implements Link {
QuiltRuleBreakOnly rule;
Option breaks;

public Breaks(Option breaks) {
public Breaks(Option breaks, QuiltRuleBreakOnly rule) {
this.breaks = breaks;
this.rule = rule;
}

public Stream<Option> children() {
Expand All @@ -335,9 +368,11 @@ public String toString() {

static class BreaksAll implements Link {
Collection<Option> breaks;
QuiltRuleBreakAll rule;

public BreaksAll(Collection<Option> breaks) {
public BreaksAll(Collection<Option> breaks, QuiltRuleBreakAll rule) {
this.breaks = breaks;
this.rule = rule;
}

public Stream<Option> children() {
Expand Down Expand Up @@ -917,7 +952,7 @@ private boolean reportBreakingMods(List<RuleLink> rootRules) {
Set<ModLoadOption> allBreakingOptions = new LinkedHashSet<>();
allBreakingOptions.addAll(ruleE.getConflictingOptions());
String reason = ruleE.publicDep.reason();
this.addError(new BreakageError(modOn, versionsOn, from, allBreakingOptions, reason));
this.addError(new BreakageError(ruleE.publicDep, from, allBreakingOptions));

return true;
}
Expand Down Expand Up @@ -1068,26 +1103,27 @@ void report(QuiltPluginManagerImpl manager) {
}

static class BreakageError extends SolverError {
final ModDependencyIdentifier modOn;
final VersionRange versionsOn;
// final ModDependencyIdentifier modOn;
// final VersionRange versionsOn;
final Set<ModLoadOption> from = new LinkedHashSet<>();
final Set<ModLoadOption> allBreakingOptions;
final String reason;
// final String reason;
final ModDependency.Only dep;

BreakageError(ModDependencyIdentifier modOn, VersionRange versionsOn, ModLoadOption from, Set<
ModLoadOption> allBreakingOptions, String reason) {
this.modOn = modOn;
this.versionsOn = versionsOn;
BreakageError(ModDependency.Only dep, ModLoadOption from, Set<
ModLoadOption> allBreakingOptions) {
this.dep = dep;
// this.versionsOn = versionsOn;
this.from.add(from);
this.allBreakingOptions = allBreakingOptions;
this.reason = reason;
// this.reason = reason;
}

@Override
boolean mergeInto(SolverError into) {
if (into instanceof BreakageError) {
BreakageError depDst = (BreakageError) into;
if (!modOn.equals(depDst.modOn) || !versionsOn.equals(depDst.versionsOn)) {
if (!dep.id().equals(depDst.dep.id()) || !dep.versionRange().equals(depDst.dep.versionRange())) {
return false;
}
depDst.from.addAll(from);
Expand All @@ -1110,7 +1146,7 @@ void report(QuiltPluginManagerImpl manager) {
String rootModName = from.size() > 1 ? from.size() + " mods" : mandatoryMod.metadata().name();

QuiltLoaderText first = VersionRangeDescriber.describe(
rootModName, versionsOn, modOn.id(), false, transitive
rootModName, dep.versionRange(), dep.id().id(), false, transitive
);

Object[] secondData = new Object[allBreakingOptions.size() == 1 ? 1 : 0];
Expand All @@ -1127,8 +1163,8 @@ void report(QuiltPluginManagerImpl manager) {

setIconFromMod(manager, mandatoryMod, error);

if (!reason.isEmpty()) {
error.appendDescription(QuiltLoaderText.translate("error.reason", reason));
if (!dep.reason().isEmpty()) {
error.appendDescription(QuiltLoaderText.translate("error.reason", dep.reason()));
// A newline after the reason was desired here, but do you think Swing loves nice things?
}

Expand Down Expand Up @@ -1159,19 +1195,39 @@ void report(QuiltPluginManagerImpl manager) {

StringBuilder report = new StringBuilder(rootModName);
report.append(" breaks");
if (VersionRange.ANY.equals(versionsOn)) {
if (VersionRange.ANY.equals(dep.versionRange())) {
report.append(" all versions of ");
} else {
report.append(" version ").append(versionsOn).append(" of ");
report.append(" version ").append(dep.versionRange()).append(" of ");
}
report.append(modOn);// TODO
report.append(dep.id());// TODO
report.append(", which is present!");
error.appendReportText(report.toString(), "");

if (!reason.isEmpty()) {
error.appendReportText("Breaking mod's reason: " + reason, "");
report = new StringBuilder();
if (dep.unless() != null) {
report.append("\n");
if (dep.unless() instanceof ModDependency.Only) {
ModDependency.Only unless = (ModDependency.Only) dep.unless();
report.append("However, if");

if (VersionRange.ANY.equals(unless.versionRange())) {
report.append(" any version of ");
} else {
report.append(" a version ").append(unless.versionRange()).append(" of ");
}

report.append(unless.id()).append(" is present, ").append(rootModName).append(" does not break ").append(dep.id()).append(".");
}
}

error.appendReportText(report.toString(), "");

if (!dep.reason().isEmpty()) {
error.appendReportText("Breaking mod's reason: " + dep.reason(), "");
}

error.appendReportText("Breaking mods: ");
for (ModLoadOption mod : from) {
error.appendReportText("- " + manager.describePath(mod.from()));
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/quiltmc/loader/impl/plugin/quilt/QuiltRuleBreakAll.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
@QuiltLoaderInternal(QuiltLoaderInternalType.NEW_INTERNAL)
public class QuiltRuleBreakAll extends QuiltRuleBreak {

final QuiltRuleBreakOnly[] options;
final ModDependency.All publicDep;
public final QuiltRuleBreakOnly[] options;
public final ModDependency.All publicDep;

public QuiltRuleBreakAll(QuiltPluginManager manager, RuleContext ctx, LoadOption option, ModDependency.All all) {

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/net/fabricmc/test/ModResolvingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ private static void resolveErrorSet(String subpath) {
fail(sb.toString());
} catch (ModResolutionException ignored) {
// Correct
ignored.printStackTrace();
System.err.println(ignored.getMessage());
}
}

Expand Down

0 comments on commit ada7ce0

Please sign in to comment.