Replace BinaryFormatter with DataContractSerializer

[MiroBrodlovaUnityGit]

Overview

BinaryFormatter isn't compatible with CoreCLR and therefore its usages need to be removed as Unity moves into CoreCLR. This PR replaces all instances of BinaryFormatter with System.Runtime.Serialization.DataContractSerializer in this repo, and is part of Epic SCP-1555.

Previous Behavior

The package used deprecated BinaryFormatter which is deprecated in future .NET versions and may prevent assemblies from unloading or return incorrect results.

New & Expected Behavior

The package now uses DataContractSerializer for serialization/deserialization with [DataContract] and [DataMember] attributes added to all serialized classes.

Changes

• KdTree.cs: Replaced BinaryFormatter with DataContractSerializer in SaveToFile and LoadFromFile methods
• KdTreeNode.cs: Added [DataContract] and [DataMember] attributes to enable serialization
• Added [DataContract] and [DataMember] attributes to all serialized classes to ensure proper serialization

Further Context

• BinaryFormatter is deprecated in future .NET versions
• Epic SCP-1555 tracks this migration work
• DataContractSerializer was chosen as it does not require referencing additional assemblies

JIRA-ticket

• SCP-1555

Testing

Running the default job-suite. Please share if there are additional steps needed!

[unity-cla-assistant]

All committers have signed the CLA.

[codecov-github-com]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff           @@
##           master     #641   +/-   ##
=======================================
  Coverage   35.57%   35.57%           
=======================================
  Files         277      277           
  Lines       34897    34897           
=======================================
  Hits        12416    12416           
  Misses      22481    22481           

Flag	Coverage Δ
mac_trunk	35.33% <ø> (-0.01%)	⬇️
win_trunk	35.23% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MiroBrodlovaUnityGit]

Marking this ready for review just for the purpose of triggering the automatic PR CI jobs (to get a gauge for how these changes fare)! Hope that's OK :)

[u-pr-agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 SCP-1555 - Partially compliant Compliant requirements: Replace/avoid CoreCLR-incompatible (“forbidden”) APIs in packages (notably AppDomain-related and other analyzer-flagged APIs). Non-compliant requirements: Ensure no packages remain that have analyzer warnings/errors for forbidden API usage (Definition of Done). Requires further human verification: Verify changes via an appropriate test plan / CI as needed. Provide/attach a technical design document if required by the epic, or state why not needed. Ensure no packages remain that have analyzer warnings/errors for forbidden API usage (Definition of Done).

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪ The diff is moderate-sized but changes serialization semantics and may require validation across multiple type combinations and runtime targets.

🏅 Score: 72 The migration removes `BinaryFormatter`, but the current `DataContractSerializer` usage on generic/internal types is likely to break without additional configuration and tests.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Serialization Breakage DataContractSerializer for KdTree<TKey, TValue> may fail at runtime because generic parameters and member types (e.g., ITypeMath<TKey>, TKey[], TValue) are not guaranteed to be known/serializable; consider providing known types, a resolver, or redesigning persistence to serialize only the data needed (points/values) rather than runtime helpers like typeMath. [DataMember] private int dimensions; [DataMember] private ITypeMath<TKey> typeMath = null; [DataMember] private KdTreeNode<TKey, TValue> root = null; [DataMember] public AddDuplicateBehavior AddDuplicateBehavior { get; private set; } public bool Add(TKey[] point, TValue value) { var nodeToAdd = new KdTreeNode<TKey, TValue>(point, value); if (root == null) DataContract Visibility Data contract serialization typically requires members/types to be serializable and often expects public accessibility or explicit attributes; the internal child links and generic node type may need [DataMember] settings (e.g., IsRequired, EmitDefaultValue) and/or making the type/public members compatible, otherwise save/load may throw. [DataContract] class KdTreeNode<TKey, TValue> { public KdTreeNode() { } public KdTreeNode(TKey[] point, TValue value) { Point = point; Value = value; } [DataMember] public TKey[] Point; [DataMember] public TValue Value = default(TValue); [DataMember] public List<TValue> Duplicates = null; [DataMember] internal KdTreeNode<TKey, TValue> LeftChild = null; [DataMember] internal KdTreeNode<TKey, TValue> RightChild = null; Contract Consistency Adding [DataMember] to Count and AddDuplicateBehavior risks persisting derived/transient state; validate that Count matches the reconstructed tree and that AddDuplicateBehavior setter-only behavior is correctly restored on deserialize. [DataMember] public int Count { get; private set; }

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[u-pr-agent]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent non-serializable strategy serialization Avoid serializing typeMath because it is typically an interface-backed strategy object and often not round-trippable with DataContractSerializer. Mark it as non-serialized and rehydrate it after load (e.g., via constructor/factory) to prevent runtime serialization failures. External/KdTree/KdTreeLib/KdTree.cs [47-48] -[DataMember] +[IgnoreDataMember] private ITypeMath<TKey> typeMath = null; Suggestion importance[1-10]: 8 __ Why: Serializing typeMath (an ITypeMath<TKey> interface strategy) is likely to fail with DataContractSerializer and also makes deserialized trees unusable without rehydrating the strategy; marking it with [IgnoreDataMember] is an important correctness fix.	Medium
More suggestions

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[varinotmUnity]

just wondering how does this work in terms of backward compatibility? For older projects tha thave saved using the old BinaryFormatter, would using DataContractSerializer work?

[MiroBrodlovaUnityGit]

This is an excellent question, thanks for asking it!

Replacing the BinaryFormatter is indeed a breaking change, or at least DataContractSerializer cannot correctly read data written by the BinaryFormatter. So old projects that have KdTree data saved using BinaryFormatter.Serialize() will not be able to load those files after this change. There are other packages that are facing the same situation, such as Addressables (you can see some of our discussion here in their PR about it)

[MiroBrodlovaUnityGit]

We've discussed this a bit within the team (I'm in Scripting Runtime), and have discussed how each team that uses BinaryFormatter will be most well-versed in the needs of their users and package, and so we leave it absolutely to your discretion how you wish to replace it of course. We need to have BinaryFormatter replaced in all packages before we'd turn on CoreCLR (as it is not compatible with CoreCLR), so it will be a blocker for CoreCLR (just sharing in case it's helpful to take into account)

What are your/your team's thoughts?