search

iminashi / Rocksmith2014.NET

F# (with some C#) libraries for creating Rocksmith 2014 custom DLC.
MIT License
67 stars 18 forks source link

IgnoreNullValues is obsolete, and does not compile after updating to … #24

Closed rickparrish closed 3 months ago

rickparrish commented 3 months ago

…the latest VS 2022 build (17.10.0)

DefaultIgnoreCondition seems to be the new property to ignore null values (https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializeroptions.ignorenullvalues?view=net-8.0)

I've only tested that the project compiles after making these changes, I'm not 100% sure that DefaultIgnoreCondition = JsonIgnoreCondition.Never is the equivalent of IgnoreNullValues = false, or that JsonIgnoreCondition.WhenWritingNull is the equivalent of true.

iminashi commented 3 months ago

Thanks. I guess some fix in the F# compiler now caused the build to fail since apparently that has been obsolete for a while now.

There are also uses in Configuration.fs that need to be fixed.

rickparrish commented 3 months ago

Thanks. I guess some fix in the F# compiler now caused the build to fail since apparently that has been obsolete for a while now.

While investigating I came across this stream that seems relevant, although I didn't actually watch it so I'm just going off the description given ("Obsolete attribute is ignored in constructor property assignment"). https://amplifyingfsharp.io/sessions/2024/03/22/

There are also uses in Configuration.fs that need to be fixed.

What would your preference there be?

let options = JsonSerializerOptions(WriteIndented = true, IgnoreNullValues = true)

to

let options = JsonSerializerOptions(WriteIndented = true, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)

or

let options = FSharpJsonOptions.Create(indent = true, ignoreNull = true)

I guess they're not quite equivalent, because the latter also does options.Converters.Add(JsonFSharpConverter())?

iminashi commented 3 months ago

What would your preference there be?

For load the options can just be removed since they shouldn't even matter there.

For save, it can be changed to let options = FSharpJsonOptions.Create(indent = true, ignoreNull = true) even though the JsonFSharpConverter isn't necessary there.

rickparrish commented 3 months ago

Sounds good to me, I've pushed those changes.