-
Notifications
You must be signed in to change notification settings - Fork 297
Allow foreign programs to discover and resolve mods without launching the game #1052
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
base: master
Are you sure you want to change the base?
Conversation
I am a bit torn between
|
I added GameDefinition so external applications can get metadata about the game itself (version, expected launch dir) to pass to the discoverer, without having to have FabricLoader.INSTANCE initialized. External applications like launchers would need to know pretty much everything in GameDefinition. Having a ModCandidateFinder for builtin mods would be nice, it should still just pull them straight from the GameDefinition though. I'm not sure what you mean with the DiscoveryContext interface. Can you give an example? |
One thing to note though, discoverMods will not return a list of mods if any throw a ModResolutionException. This would need to be changed so it's up to the implementation to crash if that is thrown. This is so launchers can still read that the mods are there, but tell the user they're broken or incompatible. I made discoverMods() return an object like this, which seems like it might be similar to the DiscoverContext you were talking about: package net.fabricmc.loader.impl.discovery;
import java.util.List;
public class ModDiscoveryInfo {
private final List<ModCandidateImpl> modsFound;
private final ModResolutionException exception;
public ModDiscoveryInfo(List<ModCandidateImpl> discoveredMods, ModResolutionException exception) {
this.modsFound = discoveredMods;
this.exception = exception;
}
public List<ModCandidateImpl> getFoundMods() {
return modsFound;
}
public ModResolutionException getException() {
return exception;
}
public boolean launchable() {
return exception == null;
}
} |
I made my own fork of fabric loader for my launcher when you said you probably weren't going to merge this. It does everything I said above. You can peruse it if you want, though you should note there are some unrelated changes for WilderForges CI. master...WilderForge:fabric-loader:master If you want, I can pick out the changes relevant to this issue and push them to my |
331b2e6
to
d442b9a
Compare
discoverMods() now returns a ModDiscoveryInfo. This object allows you to detect what mods were found, and if there are any mod resolution issues.
d442b9a
to
d3a61ca
Compare
I went ahead pushed the changes, and converted this PR into a draft, since it's mainly being used as a demonstration of the idea, not really intended to be merged as is right now (though it could be). I kinda mangled the git history when merging the main branch of my fork to this PR branch, sorry. The pr was previously at 331b2e6 |
I see,that's unexpected extra scope, which I didn't expect from the originally described purpose. How much of the information is really new to this external program?
It feels like the natural way to remove the only direct GameProvider interaction from ModDiscoverer. There is no strict need for GameDefinition with that approach.
It is one of the 3 alternatives to GameDefinition. Instead of slicing a bit off GamrProvider it's more of a minimally needed input/context for discoverer. Currently it'd only have a single method to get the builtin mods, is defined in the discoverer package and implemented by the game provider or a class near it.
This is difficult, if an error occurs it's due to some unexpected issue, not some expected incompatibility. This might be a corrupted jar, but sometimes also a weird I/O problem including the discovery timeout presumably from misbehaving AV software. The mod candidate list is IMO too unreliable to do much with at that time and incomplete. With some extra work corrupted individual mod jars/dirs could be handled and gathered along the other mods, but other error causes are hopeless. I'd probably use a ModResolutionException subtype with fields for the result data (discovered candidates, failed locations (could be nested paths!))
I wanted you to look at the extension branch with its multi-stage loading mechanism, which substantially changes the required approach and may make you want to adjust accordingly. Depending on what data you need exactly, it might be more appropriate to do almost exactly a regular launch, including running (some?) extension code, stop just shy of preLaunch, gather the final mod list and exit. We might need to add something to facilitate this kind of launch, so extensions are aware of what's going on, the launch stop at the right time and errors are conveyed differently. Extensions can effectively act as some form of ModCandidateFinder, updater, adapters to foreign ecosystems or languages etc. Without running at least the relevant extensions you'd be blind to any of their additions and changes. It is of course a bit more overhead, but as not a single game class gets class loaded I'd still count it as a prelaunching step roughly in line with what you originally mentioned? I still want to loosen the coupling of ModDiscoverer and similar parts though. Is there any reason why this wouldn't work? |
I was originally writing out detailed responses to each of your points, but during that process I realized that if extensions are going to be a part of the discovery pipeline, then a simulated launch in a separate JVM is the only viable approach. There were two main issues that led me to this colclusion: 1. Loader version mismatching:A launcher may not be using the same version of Fabric Loader as the game it’s trying to analyze. Discovery logic may also change between versions. The launcher would need to delegate to what is in the install directory. 2. Extensions can bleed static state:This is a really big issue. It's basically the same issue with public class MyExtension {
private static final Path launchDirectory; //once set, would be invalid on subsequent runs
//...other code
} and that data would be wrong on subsequent uses of the extension class on the same JVM. If extensions weren't part of the equation, I'd still argue that an implementation like this PR would be better. Ultimately, the right solution depends on if extensions are going to be used for discovery. If yes, then simulating the discovery phase in a new JVM is the correct approach. Otherwise, I still believe this PR's model would be a better fit. Regarding error handling:
My intent with that behavior was primarily to allow launchers to quickly get a list of mods for mod management purposes. Handling invalid or broken mods would be a bonus. In cases where something fails catastrophically (like a timeout or corrupted jar), the situation is probably already unrecoverable, and showing an error is good enough. I did consider adding a subtype of ModResolutionException that could carry result data, but it felt out of scope. It definitely would make more sense to add them if we're going to go through the simulation route though. |
I want to clarify, when I say "a simulated launch in a separate JVM", I just mean that each discovery attempt would have to be in a separate jvm, not that loader should be spinning up new jvms itself. The launcher can do that, I don't really see a reason for loader to do so. This would solve the FabricLoader.INSTANCE problem too, so there would be no reason to decouple anymore. |
I wasn't even considering not launching a dedicated JVM instance for this, it's really not that much overhead without a game doing its thing. |
How about this: Game Launcher (acts as a socket server):
Fabric Loader Discovery (acts as a socket client)
Loader would have to be a socket client so the launcher can be a server. It would need to manage multiple simultaneous discovery subprocesses. I had the beginnings of an example here: WilderForge@cf5baff but I didn't want to put too much effort into it if, especially if you have thought of a better way. It's just the basic structure of how the socket would work. It's not an actual implementation that works. Somewhere in FabricLoaderImpl you would have to grab the socket and then send the info. |
This PR enables tools such as custom game launchers to perform mod discovery and resolution without starting the game. This is useful for detecting mod compatibility issues pre-launch. I'd appreciate some feedback on this approach.
Note: This PR does not provide a builtin mechanism for discovering or resolving a minecraft game instance.
It only enables external tools to supply their own implementation. Such an implementation is out of scope for this PR, unless someone else wants to contribute it.
Introduced
GameDefinition
interfaceGameProvider
into a newGameDefinition
interface. This interface contains all functionality required to support mod discovery externally.GameProvider
now implementsGameDefinition
, so there will be no API breakage here.GameDefinition
. Open to naming suggestions. The interface represents a game install on the user's filesystem.Removed dependency on
FabricLoaderImpl
fromModDiscoverer
ModDiscoverer
usable by external tools. While technically a breaking change, it does not impact real-world usage. External programs could not useModDiscoverer
before due to its reliance on the internal loader instance.Introduced
ModDiscoveryInfo
ModDiscoverer.discoverMods()
no longer throwsModResolutionException
. Instead it returns an object that represents the state of the mod resolution. This is so launchers can better determine what mods are present in addition to any conflicts/issues with mods.The resulting ModResolutionException can be retrieved via
#getException()
A method called
isLaunchable()
is used to determine if the mod discovery represents a launchable game. Both loader and foreign launchers need this. Loader so it knows to crash the game, and launchers need it so they can prevent the game from launching.Tests
Example