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.

Sign up for GitHub

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

Skip serializing structs if default value #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+59 −2
Open

Skip serializing structs if default value #3

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b2de9a
Skip serialization of optional fields set to their default value
Jake-Rich May 31, 2024
2c5f5f3
Also exclude serializing default value of required fields
Jake-Rich May 31, 2024
1f15044
Add support for all default values, fix optional field checks applyin…
Jake-Rich Jun 18, 2024
ffc4649
Make sure existing null checks aren't applied on top of new default v…
Jake-Rich Jun 18, 2024
5a5747f
Don't apply to enums
Jake-Rich Dec 13, 2024
c8cb06a
Merge branch 'master' into skip_optional_default_fields
Jake-Rich Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions CodeGenerator/CodeGenerator/FieldSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,23 @@ static void BytesWriter(Field f, string stream, CodeWriter cw)
cw.WriteLine(stream + ".Write(msField.GetBuffer(), 0, (int)length" + f.ID + ");");
}

private static void WriteOptionalFieldCheck( CodeWriter cw, Field f )
{
// Support fields with custom default values
string defaultString = f.OptionDefault ?? "default";

// For some reason Ray isn't comparible so just check it's fields
if (f.ProtoTypeName == "Ray")
{
cw.IfBracket( $"instance.{f.CsName}.origin != default && instance.{f.CsName}.direction != default" );
}
Comment on lines +279 to +283
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were all the other Unity types in Common.proto tested? This could be generated rather than hard coded since all that info is there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check that it builds cause I got some random compile error trying to check if jobHandle == default. Only when building - not in the editor.

else
{
// Normally compare to default
cw.IfBracket( $"instance.{f.CsName} != {defaultString}" );
}
}

/// <summary>
/// Generates code for writing one field
/// </summary>
Expand All @@ -285,6 +302,9 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP
if ( f.ProtoType.ProtoName == ProtoBuiltin.UInt64 ) canDelta = true;
if ( f.ProtoType.ProtoName == ProtoBuiltin.Double ) canDelta = true;

// Don't change behavior of SerializeDelta() calls
bool canOptional = !hasPrevious;

if (f.Rule == FieldRule.Repeated)
{
if (f.OptionPacked == true)
Expand Down Expand Up @@ -341,12 +361,20 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP
{
if (f.ProtoType.Nullable) //Struct always exist, not optional
{
// Manual null checks above = don't do default value checks
canOptional = false;

if (f.ProtoType.ProtoName == ProtoBuiltin.Bytes && f.OptionPooled)
cw.IfBracket("instance." + f.CsName + ".Array != null");
else
cw.IfBracket("instance." + f.CsName + " != null");
}

if (canOptional)
{
WriteOptionalFieldCheck( cw, f );
}

if ( hasPrevious && canDelta )
cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName );

Expand All @@ -357,51 +385,79 @@ public static void FieldWriter(ProtoMessage m, Field f, CodeWriter cw, bool hasP

if ( hasPrevious && canDelta )
cw.EndBracket();

if (canOptional)
cw.EndBracket();
return;
}
if (f.ProtoType is ProtoEnum)
{
if (f.OptionDefault != null)
cw.IfBracket("instance." + f.CsName + " != " + f.ProtoType.CsType + "." + f.OptionDefault);
if ( canOptional)
{
WriteOptionalFieldCheck( cw, f );
}
KeyWriter("stream", f.ID, f.ProtoType.WireType, cw);
cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) );
if (f.OptionDefault != null)
cw.EndBracket();
if (canOptional)
cw.EndBracket();
return;
}

if ( hasPrevious && canDelta )
cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName );

if ( canOptional)
{
WriteOptionalFieldCheck( cw, f );
}

KeyWriter("stream", f.ID, f.ProtoType.WireType, cw);
cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) );

if ( hasPrevious && canDelta )
if ( hasPrevious && canDelta )
cw.EndBracket();
if (canOptional)
cw.EndBracket();
return;
}
else if (f.Rule == FieldRule.Required)
{
if ( hasPrevious && canDelta )
if ( hasPrevious && canDelta )
cw.IfBracket( "instance." + f.CsName + " != previous." + f.CsName );

if (f.ProtoType is ProtoMessage && f.ProtoType.OptionType != "struct" ||
f.ProtoType.ProtoName == ProtoBuiltin.String ||
f.ProtoType.ProtoName == ProtoBuiltin.Bytes)
{
// Don't do extra optional checks if it's trying to throw an error
canOptional = false;

if (f.ProtoType.ProtoName == ProtoBuiltin.Bytes && f.OptionPooled)
cw.WriteLine("if (instance." + f.CsName + ".Array == null)");
else
cw.WriteLine("if (instance." + f.CsName + " == null)");

cw.WriteIndent("throw new ArgumentNullException(\"" + f.CsName + "\", \"Required by proto specification.\");");
}

// Skip default values of fields
if ( canOptional)
{
WriteOptionalFieldCheck( cw, f );
}
KeyWriter("stream", f.ID, f.ProtoType.WireType, cw);
cw.WriteLine(FieldWriterType(f, "stream", "bw", "instance." + f.CsName, hasPrevious ) );

if ( hasPrevious && canDelta )
cw.EndBracket();

if ( canOptional )
cw.EndBracket();

return;
}
throw new NotImplementedException("Unknown rule: " + f.Rule);
Expand Down