zachsaw / MPDN_Extensions

Media Player .Net (MPDN) Open Source Extensions
Other
58 stars 18 forks source link

[Settings] Handling missing values #188

Closed Shiandow closed 9 years ago

Shiandow commented 9 years ago

Currently YaxLib interprets missing values as "null", even though it shouldn't.

It would be nice if we could do something about this because it would make it far less likely for changes to break something. At worst some settings could get lost (which is still bad, but less so).

zachsaw commented 9 years ago

I just tested this. YaxLib simply skips the property when its tag is missing in the XML file, so it'll be whatever the value is you've set in the constructor. In fact, a tag that doesn't have a value (e.g. <Options />) is treated the same.

Shiandow commented 9 years ago

Does it also work for objects that occur in a subfield of the original object? Because somehow removing the "PresetGroup" property from the SuperRes config causes it to become 'null', but possibly only if it's serialized indirectly.

zachsaw commented 9 years ago

Hmm. In that situation it seems to create a default ScriptGroup. Weird.

Shiandow commented 9 years ago

Seems we might need to dig into the YaxLib code to see what's happening. It should be possible to compile the MPDN extensions using the YaxLib source and see what's happening that way.

zachsaw commented 9 years ago

Yeah I'll take a look. I've had to make some changes to YaxLib before to fix other issues so I know where to look.

zachsaw commented 9 years ago

I've found the problem - it turns out that Yaxlib sets the member value to the default value (an object instantiated via parameterless c'tor if it's available). No idea why it was written that way to begin with though.

Shiandow commented 9 years ago

Any chance it can be fixed? It would greatly improve backwards compatibility with setting files.

zachsaw commented 9 years ago

I've fixed one problem but it seems if an inner tag is corrupt it still fails so I'll take a look at that later.

zachsaw commented 9 years ago

Test build 3550 should have this fixed.