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

Add fabric.mod.json spec from Fabric Wiki #353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Juuxel
Copy link
Member

@Juuxel Juuxel commented Jan 3, 2021

Almost a direct copy converted from Dokuwiki format to Markdown with Pandoc. The only content change is that I removed the mention of "an array containing such [mod definition] objects". However, I'm not sure if this file can be directly copied here license-wise?

Copy link
Contributor

@i509VCB i509VCB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at wiki edit history you seem to be the only editor of the page so it should be fine license wise. I will probably comb through the whole thing and mention areas where details need to be more specific.


### Mandatory fields

- **id**: Contains the mod identifier - a string value matching the `^[a-z][a-z0-9-_]{1,63}$` pattern.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't loader require mod ids are at least 3 characters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. In fact, the error messages from the parsers for missing id strings are incorrect: Loader only prevents == 1 and > 64.

if (modId.length() == 1) {
errorList.add("is only a single character! (It must be at least 2 characters long)!");
} else if (modId.length() > 64) {
errorList.add("has more than 64 characters!");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I see.


It is recommended that the keys be namespaced with the ID of the mod relying on them (if such a mod exists) to prevent conflicts.

## Version 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion v0 should have a seperate spec doc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this was just what asie had in the original. I'll remove this part.

If a "schemaVersion" key is missing, assume version "0".

It is important to remember that the Fabric mod JSON is **authoritative**. This means that Loader relies on it specifically to gather the necessary information - from the perspective of an external tool, its contents can be trusted and relied upon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably specify the behavior with regards to duplicate k->v values in the FMJ.

The way this is right now is we use the latter field EXCEPT for the schemaVersion which we fail to parse when there are multiple instances of it.

@Juuxel
Copy link
Member Author

Juuxel commented Jan 3, 2021

Looking at wiki edit history you seem to be the only editor of the page

@i509VCB That's because it's a moved page. The original page's history also shows asie and falseresync there.

@i509VCB
Copy link
Contributor

i509VCB commented Jan 3, 2021

I see that we will need to poke asie and false (done) about permission to use the spec from wiki.

I think we should get them to agree to license the spec document under CC0 (and probably the same with all the spec documents in loader for future) so we can have this spec send stuff back to the wiki? That is a question I would like to have discussed.

@falseresync
Copy link

As i5 asked, I hereby grant any and all permission to do anything and everything with this spec :>

@Juuxel
Copy link
Member Author

Juuxel commented Jan 10, 2021

@asiekierka Do you agree to licensing this spec under CC0?

Copy link

@YTG1234 YTG1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍🏻

I would like it if you added some more code blocks (e.g. ContactInformation, "schemaVersion").

@@ -0,0 +1,130 @@
# fabric.mod.json

In all cases, the mod JSON, `fabric.mod.json`, is defined as either an object containing a "schemaVersion" key with an integer value, denoting the version of the format.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either an object or...

Comment on lines +87 to +88
- **entrypoints**: Contains an EntrypointContainer. If not present, assume empty object.
- **jars**: Contains an array of NestedJarEntry objects. If not present, assume empty object.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link EntrypointContainer and NestedJarEntry to their respective sections in the markdown (no need to do if code blocks)

Comment on lines +109 to +111
- **authors**: Contains the direct authorship information - an array of Person values.
- **contributors**: Contains the contributor information - an array of Person values.
- **contact**: Contains the contact information for the project - a ContactInformation object.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to ContactInformation and Person.

@liach
Copy link
Contributor

liach commented Jun 24, 2021

@sfPlayer1 Should we include these here or just a simple readme asking people to visit fabric wiki instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants