Skip to content

Migrate to System.Text.Json#60

Open
ColinTimBarndt wants to merge 15 commits into
resonite-modding-group:mainfrom
ColinTimBarndt:feature/system-json
Open

Migrate to System.Text.Json#60
ColinTimBarndt wants to merge 15 commits into
resonite-modding-group:mainfrom
ColinTimBarndt:feature/system-json

Conversation

@ColinTimBarndt

@ColinTimBarndt ColinTimBarndt commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

Resolves #22

Notable changes:

  • Using converter factories for generic types (enums, Resonite primitives)
    • No reflection is used for the Coder<T> type anymore
  • enums have 2 generic converters: for regular enum types and for optional enums
  • ConfigurationConverter for reading and writing config files
  • ConfigurationConverter uses a thread local "context" to know what mod's config it's reading or writing
    • This was needed because System.Text.Json is a streaming API, which means that we can't just deserialize everything into a JObject and then parse it further once the version is read. With the streaming API, the config converter first reads the version field (which must come first, which it does) and then decides how to read the remaining JSON.
    • Advantage is that parsing configs should be much faster (though this has never been a problem for me)
  • Since System.Text.Json is a streaming API, the version property is expected to come before the values property. Since this is already the case, this additional requirement shouldn't cause any problems.

The general structure is there, but further polishing is needed.

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

Needs testing, but otherwise feature complete

@ColinTimBarndt ColinTimBarndt marked this pull request as ready for review January 28, 2026 22:30
@XDelta XDelta added enhancement New feature or request .NET Pull requests that update .net code labels Jan 28, 2026
DynamicJsonConverter.Write(writer, key.ValueType(), writtenValue, options);
}

writer.WriteEndObject();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Called twice?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you're referring to

@ColinTimBarndt ColinTimBarndt Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh if you're referring to ending the object twice, it's because:

{
  "version": "...",
  "values": {
    //...
  } // first
} // second

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

@XDelta is there anything left blocking this PR?

@XDelta

XDelta commented Feb 19, 2026

Copy link
Copy Markdown
Member

Pretty much just testing for breakage and what gets changed from older configs

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

I'll double check but I think I've been daily driving this custom build for some time now. Would need broader testing with other mods probably to be extra sure. Though this would break mods that implement custom serializers for Newtonsoft.Json.

@XDelta

XDelta commented Feb 20, 2026

Copy link
Copy Markdown
Member

In some initial testing, there are a few that seem to not be saving or being able to use their config. CherryTypes, ModSettings, SpecialItemsLib showed some issues in my local copy.

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

CherryTypes is using a custom Newtonsoft.Json serializer and needs to be updated, I would expect issues to arise there.

@ColinTimBarndt

ColinTimBarndt commented Feb 21, 2026

Copy link
Copy Markdown
Contributor Author

While updating CherryTyes, I found a potential issue. By default, System.Text.Json only considers public properties and not fields. We could either enable IncludeFields = true in the JsonSerializerOptions or let mod authors update their mod by either using properties instead of public fields. I'd prefer the latter because public fields are not idiomatic.

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

What errors were you getting with ModSettings? (are we using the same mod?)
I specifically tested these mods with no issues by changing config values:

  • CherryTypes (latest commit with fixes)
  • ResoniteModSettings
  • ResoniteIKCulling (has optional enum)
  • CherryPick
  • Restrainite
  • ShowDelegates

@XDelta

XDelta commented Feb 21, 2026

Copy link
Copy Markdown
Member

Im not at home but I think it was trying to do System.Type, and one other was some ValueTuple or Dict. Mainly trying to check as many as I can to address breakages where possible so that affected mods can have issues/prs created.

@ColinTimBarndt

Copy link
Copy Markdown
Contributor Author

Here's a table showing the breakages that may arise from the migration:
https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft?pivots=dotnet-10-0
System.Text.Json by design does not support "Type" to be deserialized. For stable serialization, I've made implemented my own serializer for CherryTypes since the result of ToString may change between versions of the runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request .NET Pull requests that update .net code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch from Newtonsoft.Json to System.Text.Json

2 participants