-
Notifications
You must be signed in to change notification settings - Fork 270
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
Rework mod discovery, resolution and loading #461
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,22 @@ | |
|
||
package net.fabricmc.loader.api.metadata; | ||
|
||
import java.util.Set; | ||
import java.util.Collection; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
import net.fabricmc.loader.api.Version; | ||
import net.fabricmc.loader.api.VersionPredicate; | ||
import net.fabricmc.loader.api.metadata.version.VersionPredicate; | ||
|
||
/** | ||
* Represents a dependency. | ||
*/ | ||
public interface ModDependency { | ||
/** | ||
* Get the kind of dependency. | ||
*/ | ||
Kind getKind(); | ||
|
||
/** | ||
* Returns the ID of the mod to check. | ||
*/ | ||
|
@@ -42,5 +49,63 @@ public interface ModDependency { | |
* | ||
* @return representation of the dependency's version requirements | ||
*/ | ||
Set<VersionPredicate> getVersionRequirements(); | ||
Collection<VersionPredicate> getVersionRequirements(); | ||
|
||
enum Kind { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a find of 'kind'. Would 'type', 'relation', or 'relationship' be better here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "type" is too overloaded with other meanings in this concept, relation* is wrong since ModDependency itself is the relation. Not sure what's wrong with calling depends, recommends , ... kinds of dependencies. |
||
DEPENDS("depends", true, false), | ||
RECOMMENDS("recommends", true, true), | ||
SUGGESTS("suggests", true, true), | ||
CONFLICTS("conflicts", false, true), | ||
BREAKS("breaks", false, false); | ||
|
||
private static final Map<String, Kind> map = createMap(); | ||
private final String key; | ||
private final boolean positive; | ||
private final boolean soft; | ||
|
||
Kind(String key, boolean positive, boolean soft) { | ||
this.key = key; | ||
this.positive = positive; | ||
this.soft = soft; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd document this better and give it a more meaningful name to clarify that dependency kinds that are soft only produce warnings rather than failing mod resolution. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's to indicate it being a soft dependency (opposite of a hard dependency), whether it produces a warning is unspecified (suggests doesn't, recommends/conflicts do) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is a hard dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hard = must be met, soft = should be met |
||
} | ||
|
||
/** | ||
* Get the key for the dependency as used by fabric.mod.json (v1+) and dependency overrides. | ||
*/ | ||
public String getKey() { | ||
return key; | ||
} | ||
|
||
/** | ||
* Get whether the dependency is positive, encouraging the inclusion of a mod instead of negative/discouraging. | ||
*/ | ||
public boolean isPositive() { | ||
return positive; | ||
} | ||
|
||
/** | ||
* Get whether it is a soft dependency, allowing the mod to still load if the dependency is unmet. | ||
*/ | ||
public boolean isSoft() { | ||
return soft; | ||
} | ||
|
||
/** | ||
* Parse a dependency kind from its key as provided by {@link #getKey}. | ||
*/ | ||
public static Kind parse(String key) { | ||
return map.get(key); | ||
} | ||
|
||
private static Map<String, Kind> createMap() { | ||
Kind[] values = values(); | ||
Map<String, Kind> ret = new HashMap<>(values.length); | ||
|
||
for (Kind kind : values) { | ||
ret.put(kind.key, kind); | ||
} | ||
|
||
return ret; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any concern here from potential existing implementations which wouldn't implement
Comparable
? I can't think of any but it is technically a backward compatibility problemThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, are users even supposed to implement this interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They shouldn't need to, but that hasn't stopped people doing odd things with internals in the past
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really supported and somewhat pointless (can't put that
Version
anywhere in loader). The JVM doesn't verify whether a class implements everything, so would only throw AbstractMethodError on access.