FabricMC / fabric-loader

Fabric's mostly-version-independent mod loader.
Apache License 2.0
632 stars 269 forks source link

Properly defining a mod metadata format #70

Closed asiekierka closed 5 years ago

asiekierka commented 5 years ago

As you might've noticed, in 0.1.x-0.3.x the fabric.mod.json format is effectively "defined" by the Java class - this doesn't really make for good documentation or specification.

This is an issue opened to discuss things which should be covered by a fabric.mod.json standard ("schemaVersion": 1) which I hope to add in 0.4.0 - with the prior JSON files being implied as "schemaVersion": 0.

These are the existing fields in the mod.json format as of 0.3.6:

    // Required
    private String id;
    private String version;

    // Optional (environment)
    private DependencyMap requires = new DependencyMap();
    private DependencyMap conflicts = new DependencyMap();
    private String languageAdapter = "net.fabricmc.loader.language.JavaLanguageAdapter";
    private Mixins mixins = Mixins.EMPTY;
    private Side side = Side.UNIVERSAL;
    private boolean lazilyLoaded = false;
    private String initializer;
    private String[] initializers;

    // Optional (metadata)
    private String name;
    private String description = "";
    private Links links = Links.EMPTY;
    private DependencyMap recommends = new DependencyMap();
    private Person[] authors = new Person[0];
    private Person[] contributors = new Person[0];
    private String license = "";

Some prior art:

I'd also like to call in @peterix as he might have lots of prior art from his work on MultiMC.

asiekierka commented 5 years ago

Versioned API dependencies are necessary IMO.

APIs are meant to be separate JARs. This is for a much smaller edge case - also, after discussion with Grondag (the main user of the feature so far), we decided to add a case-specific kludge via "custom" and delegate this to 0.5.0, as to not delay/break the mess even further.

Alternatively, could we compute dependencies once at first startup of an instance, and only recompute them when the mod list or a mod's version changes?

This is under consideration. There's already a big simplification in place - nested JARs are ignored if the same mod ID is provided by an end user.

asiekierka commented 5 years ago

https://fabricmc.net/wiki/format:modjson

Updated with InitializerEntry spec. Time to implement...

Prospector commented 5 years ago

Would very much like Environment to be exposed in the api's ModMetadata as well

asiekierka commented 5 years ago

It is.

Okay, we're done! Nobody responded in a month, so that means you're all okay with it, right? Right?

Shipping Loader 0.4.0 ASAP. Bye!

asiekierka commented 5 years ago

OH NO WE'RE NOT DONE YET

Most people, myself included, would prefer a more verbose but extensible/unambiguous entrypoint definition system:

"entrypoints": {
    "main": {
        {
            "value": "my/package/MyClass::myField"
        },
        {
            "adapter": "scala",
            "value": "whatever_scala_wants"
        }
    }
}

This would allow unambiguous parsing and easy extension and no indexOf nonsense, but make things a lot more painful to wade through at tens of initializers. I'd consider allowing skipping {"value"} single-keyed objects and allowing passing strings directly, myself, but that's me.

asiekierka commented 5 years ago

Should LanguageAdapters be fed the whole JsonObject, while at it? I don't like this, as it enforces a Gson dependency on the mod metadata API - but maybe there's a better way than solely passing them one string? (I'd say leave this for schemaVersion v2, and potential future breaking changes...)

asiekierka commented 5 years ago

Idea: Pass a Map<String, String> with all the non-"adapter" keys to the language adapter, and for "default" use the following:

as valid keys.

Juuxel commented 5 years ago
"entrypoints": {
   "main": {
       {
           "value": "my/package/MyClass::myField"
       },
       {
           "adapter": "scala",
           "value": "whatever_scala_wants"
       }
   }
}

Using simple strings for value-only objects would make the format simpler in my opinion.

For example:

"entrypoints": { "main": [ "my/package/MyClass" ] }
asiekierka commented 5 years ago

We still need the adapter info.

Here's a new proposal:

"entrypoints": [
    {
        "type": "main",
        "adapter": "default",
        "values": [
            "my/package/MyClass",
            "my/package/MyOtherClass::myMethod"
        ]
    }
}
modmuss50 commented 5 years ago

I think thats good, still allow for more than one lang adapter per mod, but also doesn't take 3 lines per value.

What about not needing to have the default values?

asiekierka commented 5 years ago

Well, "adapter" is optional. The rest is mandatory.

asiekierka commented 5 years ago

Player's compromise

"entrypoints": {
    "main": [
        "my/package/MyClass",
        {
            "adapter": "not_default",
            "value": "my/package/MyOtherClass::myMethod"
        }
    ]
}

The main drawback it has is defining the adapter every time you have a different entrypoint, but this only comes into play when you have a custom language plus multiple entrypoints.

Thoughts?

asiekierka commented 5 years ago

Yeah that last one