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

Handle removing mod load options properly #288

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ default <V> QuiltPluginTask<V> addGuiRequest() {
* {@link QuiltLoaderPlugin#resolve(QuiltPluginContext, TentativeLoadOption)}, if it is selected.
*
* @param option */
<T extends LoadOption & TentativeLoadOption> void addTentativeOption(T option);
<T extends LoadOption & TentativeLoadOption> void addTentativeOption(T option, Class<T> clazz);

/** Only callable during {@link QuiltLoaderPlugin#handleError(java.util.List)} to identify the given rule as one
* which can be removed for the purposes of error message generation. */
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/quiltmc/loader/api/plugin/solver/Rule.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ public abstract class Rule {
public Rule() {}

/** @return true if {@link #define(RuleDefiner)} needs to be called again, or false if the added option had no
* affect on this rule. */
* effect on this rule. */
public abstract boolean onLoadOptionAdded(LoadOption option);

/** @return true if {@link #define(RuleDefiner)} needs to be called again, or false if the removed option had no
* affect on this rule. */
* effect on this rule. */
public abstract boolean onLoadOptionRemoved(LoadOption option);

/** Called whenever a {@link LoadOption} is changed. Not all {@link Rule}s are expected to be able to update to all
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void addModLoadOption(ModLoadOption mod, PluginGuiTreeNode guiNode) {
}

@Override
public <T extends LoadOption & TentativeLoadOption> void addTentativeOption(T option) {
public <T extends LoadOption & TentativeLoadOption> void addTentativeOption(T option, Class<T> clazz) {
addTentativeOption0(option);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
@QuiltLoaderInternal(QuiltLoaderInternalType.NEW_INTERNAL)
public final class MandatoryModIdDefinition extends ModIdDefinition {
public final ModLoadOption option;
private boolean optionGone = false;

public MandatoryModIdDefinition(ModLoadOption candidate) {
this.option = candidate;
Expand All @@ -45,7 +46,9 @@ ModLoadOption[] sources() {

@Override
public void define(RuleDefiner definer) {
definer.atLeastOneOf(option);
if (!optionGone) {
definer.atLeastOneOf(option);
}
}

@Override
Expand All @@ -55,7 +58,7 @@ public boolean onLoadOptionAdded(LoadOption option) {

@Override
public boolean onLoadOptionRemoved(LoadOption option) {
return false;
return optionGone = option == this.option;
}

@Override
Expand All @@ -74,5 +77,8 @@ public void fallbackErrorDescription(StringBuilder errors) {
errors.append(getFriendlyName());
errors.append(" v");
errors.append(option.metadata().version());
if (optionGone) {
errors.append(" (gone)");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ private void recalculateWeights() {

@Override
public void define(RuleDefiner definer) {
if (sources.isEmpty()) {
return;
}

boolean anyAreAlways = false;

for (ModLoadOption mod : sources) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class StandardQuiltPlugin extends BuiltinQuiltPlugin {

private QuiltOverrides overrides;
private final Map<String, OptionalModIdDefintion> modDefinitions = new HashMap<>();
private final Map<ModLoadOption, List<ModLoadOption>> providedMods = new HashMap<>();

@Override
public void load(QuiltPluginContext context, Map<String, LoaderValue> previousData) {
Expand Down Expand Up @@ -327,7 +328,9 @@ public void onLoadOptionAdded(LoadOption option) {
PluginGuiTreeNode guiNode = context().manager().getGuiNode(mod)//
.addChild(QuiltLoaderText.translate("gui.text.providing", provided.id()));
guiNode.mainIcon(guiNode.manager().iconUnknownFile());
context().addModLoadOption(new ProvidedModOption(mod, provided), guiNode);
ProvidedModOption providedOption = new ProvidedModOption(mod, provided);
providedMods.computeIfAbsent(mod, i -> new ArrayList<>(provides.size())).add(providedOption);
context().addModLoadOption(providedOption, guiNode);
}
}

Expand Down Expand Up @@ -364,7 +367,19 @@ public void onLoadOptionAdded(LoadOption option) {
}
}
}

}
}

@Override
public void onLoadOptionRemoved(LoadOption option) {
// remove provided options
if (providedMods.containsKey(option)) {
for (ModLoadOption modLoadOption : providedMods.get(option)) {
context().ruleContext().removeOption(modLoadOption);
}
}
providedMods.remove(option);
}

private static void warn(String msg) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ public void addOption(LoadOption option) {
}

for (Rule rule : rulesToRedefine) {
if (LOG) {
Log.info(CATEGORY, "Redefining rule " + rule + " because of added load option" + option);
}
redefine(rule);
}
}
Expand Down Expand Up @@ -197,6 +200,9 @@ public void removeOption(LoadOption option) {
}

for (Rule rule : rulesToRedefine) {
if (LOG) {
Log.info(CATEGORY, "Redefining rule " + rule + " because of removed load option " + option);
}
redefine(rule);
}
}
Expand Down Expand Up @@ -235,7 +241,7 @@ public void removeRule(Rule rule) {
public void redefine(Rule rule) {

if (LOG) {
Log.info(CATEGORY, "Redefining rule " + rule);
Log.info(CATEGORY, "Doing redefine of rule " + rule);
}

validateCanAdd();
Expand Down