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.

Sign up for GitHub

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

Jump to bottom

Add config schema generation #930

Closed
wants to merge 5 commits into from
Closed

Add config schema generation #930

wants to merge 5 commits into from

Conversation

erri120
Copy link

@erri120 erri120 commented Feb 26, 2024 •
edited
Loading

Resolves #928.

Changes

  • Adds a new dependency to SMAPI.Toolkit: NJsonSchema for schema generation.
  • Adds GenerateConfigSchema option to SConfig that's set to false by default.
  • Adds IDataHelper.WriteJsonSchemaFile and JsonHelper.WriteJsonSchemaFile to generate and write the schema to file.
  • Updates ModHelper.ReadConfig to generate the file config.schema.json.

Notes

I think develop references DLLs from Stardew Valley 1.6 that I don't have access with, so I had to create the code on stable and port it to develop.

I'm not sure how to test this PR, there aren't any unit tests that I can run. I thought about installing latest SMAPI normally and just overwriting the DLLs in the game folder with the ones I build using the PR. However, I don't even know if that works with the .NET DLL resolution process...

@Shockah
Copy link
Contributor

Shockah commented Feb 26, 2024

You'd be correct in saying that develop references Stardew Valley 1.6. Getting access is as simple as joining the Stardew Valley Discord server and asking in the #making-mods channel to get added (not even pinging anyone - anyone already in the alpha can add you).

@Pathoschild Pathoschild force-pushed the develop branch 2 times, most recently from 4c7eac9 to 8837303 Compare March 19, 2024 04:30
@Pathoschild Pathoschild added this to the 4.1.x milestone Mar 29, 2024
@Pathoschild Pathoschild force-pushed the develop branch 2 times, most recently from e201c96 to 3793d2e Compare May 31, 2024 03:55
@Pathoschild
Copy link
Owner

Thanks! Unfortunately Json.NET Schema isn't free, and the AGPL version is limited to 1000 validations per hour. That could easily be reached in our case (since players can have hundreds or thousands of mods, they may launch the game more than once per hour, and mods often read/write their config files more than once).

@Pathoschild Pathoschild marked this pull request as draft July 7, 2024 04:25
@erri120
Copy link
Author

erri120 commented Jul 7, 2024

Updated PR to use NJsonSchema.

erri120 and others added 5 commits July 7, 2024 11:58
@Pathoschild Pathoschild force-pushed the feat/928-config-schema-generation-develop branch from 052f252 to 130fdba Compare July 7, 2024 16:08
@Pathoschild
Copy link
Owner

Pathoschild commented Jul 7, 2024 •
edited
Loading

Unfortunately NJsonSchema fails for many mods (see log). A few sample cases:

mod config result
Automate config ❌ crashes with InvalidOperationException.
Chests Anywhere config ❌ ditto Automate.
CJB Cheats Menu config ❌ ditto Automate.
CJB Item Spawner config ❌ ditto Automate.
CJB Show Item Sell Price config ❌ creates a schema for an empty object: { "$schema": "...", title: "ModConfig", type: "object", "additionalProperties": false }.
Content Patcher config ❌ ditto Automate.
Crops Anytime Anywhere config ✅ Seems correct.
Data Layers config ❌ ditto Automate.
Debug Mode config ❌ ditto Automate.
Fast Animations config ✅ Seems correct.
Generic Mod Config Menu config ❌ only includes ScrollSpeed property.
Horse Flute Anywhere config ❌ ditto Automate.
NPC Location Compass config ✅ Seems correct.
Skip Intro config ✅ Seems correct.

It seems to only work with properties (not fields), and fails with some common types like KeybindList (maybe due to the get-only properties?).

I rebased the PR branch onto the latest develop, so you can try it like this:

  1. Reset to the latest PR branch.
  2. Open SMAPI in Visual Studio.
  3. In SMAPI/Constants.cs, change the MinimumGameVersion back to 1.6.8.
  4. Click Build > Rebuild Solution to deploy the SMAPI files.
  5. Click Debug > Start Without Debugging to launch SMAPI.

@Pathoschild
Copy link
Owner

I'll close this as won't do per the above, but feel free to submit a new PR if you can get NJsonSchema or another package to work. Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.1.x
Development

Successfully merging this pull request may close these issues.

Generate config.schema.json file for validation inside mod managers
3 participants
@erri120 @Shockah @Pathoschild