Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert the array index addition to property names #183

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adainrivers
Copy link
Contributor

These changes made it impossible to deserialize the JSONs in typed languages. If you have an alternative suggestion, happy to implement them.

@GMatrixGames
Copy link
Collaborator

Alternative would be to write these c-style arrays as json arrays, where the indexes without values would be filled with serialized null values. Honestly, I'd prefer that over these strings.

@adainrivers
Copy link
Contributor Author

I'm not sure why the properties have array indexes to begin with, so you lost me there.

@GMatrixGames
Copy link
Collaborator

In unreal, you have static length, or c-style, arrays (UObject[]), then there's dynamic length arrays, (TArray). TArrays are serialized with a length and each object is serialized sequentially. c-style arrays serialize as a property with an index for its position in the array. If there's a property at index 0 and 1, they'll both be serialized as the same property name, causing issues as well.

@4sval
Copy link
Collaborator

4sval commented Dec 31, 2024

#158 for reference

@adainrivers
Copy link
Contributor Author

Updated, now the props serialize as array if they have an HasArrayIndex flag or multiple matches by name (as fallback)

@adainrivers
Copy link
Contributor Author

Alternatively, I can also serialize them as dictionaries, where the array index is the key, lmk.

@GMatrixGames
Copy link
Collaborator

Using a for-each doesn't allow for serializing the null values to fill the array.

@adainrivers
Copy link
Contributor Author

Can find the max index and fill the non-existing ones with nulls

@GMatrixGames
Copy link
Collaborator

image
This was my kind of solution when I was testing this out. This looks for one property, but can probably be adapted and improved.

@adainrivers
Copy link
Contributor Author

adainrivers commented Jan 1, 2025

  writer.WritePropertyName("Properties");
  writer.WriteStartObject();

  var properties = Properties.GroupBy(p => p.Name.Text).ToDictionary(p => p.Key, p => p.ToDictionary(pp => pp.ArrayIndex));

  foreach (var (name, props) in properties)
  {
      writer.WritePropertyName(name);

      var firstProp = props.Values.First();

      if (firstProp.PropertyTagFlags.HasFlag(EPropertyTagFlags.HasArrayIndex) || props.Count > 1)
      {
          var maxIndex = props.Keys.Max();
          writer.WriteStartArray();
          for(var i = 0; i <= maxIndex; i++)
          {
              if (!props.TryGetValue(i, out var prop))
              {
                  writer.WriteNull();
              }
              else
              {
                  serializer.Serialize(writer, prop.Tag);
              }
          }
          writer.WriteEndArray();
      }
      else
      {
          serializer.Serialize(writer, firstProp.Tag);
      }
  }

  writer.WriteEndObject();

how about this?

@adainrivers
Copy link
Contributor Author

I don't know if there is a better way to get the array size, this is an example output

      "BlendParameters": [
        {
          "DisplayName": "MoveRight",
          "Min": -30.0,
          "Max": 30.0
        },
        {
          "DisplayName": "MoveForward",
          "Min": -30.0,
          "Max": 30.0
        }
      ],

@adainrivers
Copy link
Contributor Author

It's even more messed up, there are occurrences where same property name and array index exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants