Added DataTreeConverter tests by Earthmark · Pull Request #32 · Yellow-Dog-Man/Elements.Core

Earthmark · 2026-08-01T01:49:09Z

Added initial tests for the data tree, these avoid file access.

Also added IEnumerable to DataTreeDictionary, this allows collection initializers on the object, making test cases easier to generate.

Before

var element = new DataTreeDictionary();

element.Add("Id", i);
element.Add("Name", "element-" + i);
element.Add("Position", new float3(i * 0.5f, i * 0.25f, i * 0.125f));
element.Add("Enabled", (i & 1) == 0);
element.Add("Weight", i * 1.5);

After

var element = new DataTreeDictionary
{
    { "Id", i },
    { "Name", "element-" + i },
    { "Position", new float3(i * 0.5f, i * 0.25f, i * 0.125f) },
    { "Enabled", (i & 1) == 0 },
    { "Weight", i * 1.5 }
}

Enumeration is for the keys on that level of the dictionary, EnumerateTree is for recursion.

Earthmark · 2026-08-01T02:06:50Z

~~As with #28 , once I hear back about what the test format is supposed to be, I'll update the test names to match the expected style. For now they're in the style of the local repo.~~

This was lightly discussed in Yellow-Dog-Man/Elements.Style#11 , the takeaway was there isn't really a concrete existing style. The pr was adjusted to follow the standard .net test style.

Earthmark · 2026-08-01T05:26:05Z

+    }
+
+    [TestMethod]
+    public void EnumerateKnownChildSequence()


This was the test added to verify #33 , this is possibly not a useful test though. Some of the collections being enumerated are dictionaries.

If the order isn't important this can be changed to just ensure all items are in the enumeration, instead of caring about the order.

Earthmark · 2026-08-01T19:31:50Z

More tests around historic file parsing should be added in the future, a lot of the DataTreeConverter is repeated sections of file saving code, but it's difficult to simplify without historic edge cases.

Frooxius · 2026-08-07T03:44:58Z

+    [DataRow("ASSET.LZ4BSON", true)]
+    [DataRow("asset.bson", false)]
+    [DataRow("asset.json", false)]
+    [DataRow("asset.frdt", false)] // TODO: This should be true, and is probably a bug.


We generally don't use uncompressed versions, so I think it's ok.

Frooxius · 2026-08-07T03:47:45Z

+    {
+        get
+        {
+            // Commented types do not come through as the same type, ints are promoted to longs, floats to doubles.


Hmm... is this something that can also be replicate with Resonite itself? Say you create ValueField and put MaxValue in it, save it and then it doesn't load correctly?

In resonite it appears to work correctly,

In my own testing though, if I turn that off the value that comes back as -1 (int64). I'm assuming some layer converting from the data tree to the component type does a reinterpert cast?

So at the data tree layer this looks suspect as heck, but higher up the type gets converted back so it's fine.

As per the discussion in #39, I'll see about swapping this to Verify to see if this is a bit easier to understand. That may express the long-double conversion more easily.

Turns out this gets messy, Verify doesn't include type names on the value fields, even if settings.AddExtraSettings(s => s.TypeNameHandling = TypeNameHandling.All|Auto) is used. It only includes type names on the object types.

For now I'll leave this as is then, I personally feel it's important to show the type-promotion, but I can shift it in either direction if poked.

Another test should be added at some point anyways to verify the Add methods all work.

Also added IEnumerable to DataTreeDictionary, this allows collection initializers on the object, making test cases easier to generate. Before ```csharp var element = new DataTreeDictionary(); element.Add("Id", i); element.Add("Name", "element-" + i); element.Add("Position", new float3(i * 0.5f, i * 0.25f, i * 0.125f)); element.Add("Enabled", (i & 1) == 0); element.Add("Weight", i * 1.5); ``` After ```csharp var element = new DataTreeDictionary { { "Id", i }, { "Name", "element-" + i }, { "Position", new float3(i * 0.5f, i * 0.25f, i * 0.125f) }, { "Enabled", (i & 1) == 0 }, { "Weight", i * 1.5 } } ``` Enumeration is for the keys on that level of the dictionary, EnumerateTree is for recursion.

Earthmark · 2026-08-13T07:21:00Z

This got a rather massive refresh, effectively consider it a new review.

I actually pointed claude at being a reviewer and it was quite unhappy with my lack of case covering; especially when told to follow the standard.

This now adds a few other test files as well, but I can split them off if requested.

Earthmark · 2026-08-13T08:44:57Z

+            None = 0,
+            LZ4 = 1,
+            LZMA = 2,
+            Brotli = 3,


I noticed these were not tagged, but if this order changed or was added in the middle, this would possibly break all manifests. So it's a bit stronger now!

This should probably get a comment in the source.

Earthmark · 2026-08-13T08:47:45Z

+
+    #region Serialization Unstable Trees
+
+    public static IEnumerable<TreeCase> HappyLittleSerializationUnstableTrees


I was getting frustrated at seeing the unused test sections due to their serialization stability, so the fixture below was added. And in that a few odd things fell out like the ulong extract thing.

Additional tests should be added to verify more vector types, and the rest of the generated types probably.

Earthmark · 2026-08-13T08:49:41Z

+            { "NullString", null as string },
+            { "Url", new Uri("https://example.com/asset") },
+            // NOTE: This stores the enum as a string, a hard-typed extractor is only able to unpack this correctly.
+            { "Enum", DataTreeConverter.Compression.LZMA },


This one was extra strange, the add call formats this to a string, but you can create a DataTreeValue holding an enum directly that's invalid.

I didn't test the invalid case, I hope it doesn't come up in prod

Earthmark · 2026-08-13T08:50:50Z

+    /// </summary>
+    static TreeCase VeryDeeplyNestedTree()
+    {
+        // Somewhere between 1000 and 2000 causes a stack overflow on save.


The recursion limit should probably be re-enstated, just at around a larger limit. Stack overflows are scary

Earthmark · 2026-08-13T08:52:08Z

        {
            var ext = Path.GetExtension(file).ToLower();

-            return ext == ".7zbson" || ext == ".lz4bson" || ext == ".brson" || ext == "frdt";


This was a bug that could never be hit. The above get extension call always prefixed the string with a ".".

Earthmark · 2026-08-13T08:53:36Z

 namespace Elements.Core
 {
-    public partial class DataTreeDictionary : DataTreeNode
+    public partial class DataTreeDictionary : DataTreeNode, IEnumerable<KeyValuePair<string, DataTreeNode>>


This allows collection initializers, which was real useful as the add calls are strongly typed. This made building test cases much cleaner.

Earthmark · 2026-08-13T08:55:18Z

+        var expected = DataTreeConverter.Compression.Brotli;
+        var dict = new DataTreeDictionary
+        {
+            { "Enum", expected }


This needed to be added to a dictionary because the dictionary add method contains the logic to convert the enum to the DataTreeValue (which is a string conversion). Extracting it back is on the leaf.

If you build the leaf by the enum, then extract it the extract throws.

Earthmark · 2026-08-13T08:57:19Z

+        // This is sketchy, but work with me...
+        // The extract method does type conversions that are non-trivial.
+        // We don't care how they're done, just that they're 1-1.
+        var method = typeof(DataTreeValue).GetMethod(nameof(DataTreeValue.Extract), types: [])!


These get cached to make it a bit faster. It's still not great but I couldn't think of another way without this being messy as heck.

There might be a way to do this with dynamics, but I didn't wanna look into loading that for one test.

Earthmark · 2026-08-13T09:02:59Z

+    /// </summary>
+    [DynamicData(nameof(HappyLittleSerialStableTrees))]
+    [TestMethod]
+    public void SaveLoad_StableTree_TreeSerializesIdentically(TreeCase treeCase)


With assert unstable, I don't think this test is actually testing something we care about anymore.

This verifies the internal format is stable, but we don't really care about that.

If this goes away though, some of the accessors on the nodes should probably be locked down.

Earthmark · 2026-08-14T04:48:58Z

I just realized Extract is largely called from LoadSaveHelper... there's at least three layers of public interfaces on this thing :|

Is there a specific layer that external APIs use, or do they use the whole layer cake? If possible I'd like to pull the tests up to whatever layer is used externally; or at least some of the non-exposed layers can be isolated instead.

EDIT: Coder also wraps this interface in some of its calls, that makes 4 layers.

EDIT 2: I did some digging and this layer is used during slot saving at the very least; these tests do cover the interface at a level prod code uses.

ProbablePrime · 2026-08-26T04:04:02Z

As Froox had left comments we should await a response, I will tag him for a review.

Earthmark mentioned this pull request Aug 1, 2026

Enumerate Tree Performance Boost #33

Open

Earthmark force-pushed the enumerable-dict branch from 2adc2b4 to 357ab25 Compare August 1, 2026 05:12

Earthmark commented Aug 1, 2026

View reviewed changes

Earthmark force-pushed the enumerable-dict branch from 357ab25 to 59c3fff Compare August 2, 2026 02:17

Earthmark mentioned this pull request Aug 2, 2026

DataTreeConverter Cleanup Question #43

Open

Earthmark force-pushed the enumerable-dict branch 6 times, most recently from bac612f to 284d677 Compare August 5, 2026 22:31

Frooxius reviewed Aug 7, 2026

View reviewed changes

Earthmark force-pushed the enumerable-dict branch from 284d677 to f51f092 Compare August 13, 2026 07:20

Earthmark commented Aug 13, 2026

View reviewed changes

ProbablePrime requested a review from Frooxius August 26, 2026 04:04

ProbablePrime assigned Earthmark Aug 26, 2026


		#region Serialization Unstable Trees

		public static IEnumerable<TreeCase> HappyLittleSerializationUnstableTrees

Uh oh!

Uh oh!

Conversation

Earthmark commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Earthmark commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earthmark commented Aug 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earthmark Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earthmark commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earthmark commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ProbablePrime commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Earthmark commented Aug 1, 2026 •

edited

Loading

Earthmark commented Aug 1, 2026 •

edited

Loading

Earthmark Aug 7, 2026 •

edited

Loading

Earthmark commented Aug 13, 2026 •

edited

Loading

Earthmark commented Aug 14, 2026 •

edited

Loading