From 4818c3aab4dc09a836cee0f998fbedbc452050d9 Mon Sep 17 00:00:00 2001 From: DrSmugleaf Date: Fri, 29 Sep 2023 22:14:10 -0700 Subject: [PATCH] Make auto comp states infer when data should be cloned (#4461) --- RELEASE-NOTES.md | 2 +- .../ComponentNetworkGenerator.cs | 80 ++++++++++++------- .../Robust.Shared.CompNetworkGenerator.csproj | 1 + .../ComponentNetworkGeneratorAuxiliary.cs | 14 ---- 4 files changed, 55 insertions(+), 42 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index e9907dbd64d..ac7ad5ff827 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -10,7 +10,7 @@ Don't change the format without looking at the script! ### Breaking changes -*None yet* +* Removed cloneData from AutoNetworkedFieldAttribute. This is now automatically inferred. ### New features diff --git a/Robust.Shared.CompNetworkGenerator/ComponentNetworkGenerator.cs b/Robust.Shared.CompNetworkGenerator/ComponentNetworkGenerator.cs index 86f4a8609f6..f51722c1d9a 100644 --- a/Robust.Shared.CompNetworkGenerator/ComponentNetworkGenerator.cs +++ b/Robust.Shared.CompNetworkGenerator/ComponentNetworkGenerator.cs @@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; +using static Microsoft.CodeAnalysis.SymbolDisplayFormat; namespace Robust.Shared.CompNetworkGenerator { @@ -14,15 +15,23 @@ public class ComponentNetworkGenerator : ISourceGenerator { private const string ClassAttributeName = "Robust.Shared.Analyzers.AutoGenerateComponentStateAttribute"; private const string MemberAttributeName = "Robust.Shared.Analyzers.AutoNetworkedFieldAttribute"; + private const string GlobalEntityUidName = "global::Robust.Shared.GameObjects.EntityUid"; private const string GlobalNullableEntityUidName = "global::Robust.Shared.GameObjects.EntityUid?"; + private const string GlobalEntityCoordinatesName = "global::Robust.Shared.Map.EntityCoordinates"; private const string GlobalNullableEntityCoordinatesName = "global::Robust.Shared.Map.EntityCoordinates?"; + private const string GlobalEntityUidSetName = "global::System.Collections.Generic.HashSet"; private const string GlobalNetEntityUidSetName = "global::System.Collections.Generic.HashSet"; + private const string GlobalEntityUidListName = "global::System.Collections.Generic.List"; private const string GlobalNetEntityUidListName = "global::System.Collections.Generic.List"; + private const string GlobalDictionaryName = "global::System.Collections.Generic.Dictionary"; + private const string GlobalHashSetName = "global::System.Collections.Generic.HashSet"; + private const string GlobalListName = "global::System.Collections.Generic.List"; + private static string GenerateSource(in GeneratorExecutionContext context, INamedTypeSymbol classSymbol, CSharpCompilation comp, bool raiseAfterAutoHandle) { var nameSpace = classSymbol.ContainingNamespace.ToDisplayString(); @@ -30,7 +39,7 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName var stateName = $"{componentName}_AutoState"; var members = classSymbol.GetMembers(); - var fields = new List<(ITypeSymbol Type, string FieldName, AttributeData Attribute)>(); + var fields = new List<(ITypeSymbol Type, string FieldName)>(); var fieldAttr = comp.GetTypeByMetadataName(MemberAttributeName); foreach (var mem in members) @@ -47,7 +56,7 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName switch (mem) { case IFieldSymbol field: - fields.Add((field.Type, field.Name, attribute)); + fields.Add((field.Type, field.Name)); break; case IPropertySymbol prop: { @@ -83,7 +92,7 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName continue; } - fields.Add((prop.Type, prop.Name, attribute)); + fields.Add((prop.Type, prop.Name)); break; } } @@ -121,9 +130,9 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName // component.Count = state.Count; var handleStateSetters = new StringBuilder(); - foreach (var (type, name, attribute) in fields) + foreach (var (type, name) in fields) { - var typeDisplayStr = type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); + var typeDisplayStr = type.ToDisplayString(FullyQualifiedFormat); var nullable = type.NullableAnnotation == NullableAnnotation.Annotated; var nullableAnnotation = nullable ? "?" : string.Empty; @@ -150,12 +159,32 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName handleStateSetters.Append($@" component.{name} = EnsureCoordinates<{componentName}>(state.{name}, uid);"); + break; + case GlobalEntityUidSetName: + stateFields.Append($@" + public {GlobalNetEntityUidSetName} {name} = default!;"); + + getStateInit.Append($@" + {name} = GetNetEntitySet(component.{name}),"); + handleStateSetters.Append($@" + component.{name} = EnsureEntitySet<{componentName}>(state.{name}, uid);"); + + break; + case GlobalEntityUidListName: + stateFields.Append($@" + public {GlobalNetEntityUidListName} {name} = default!;"); + + getStateInit.Append($@" + {name} = GetNetEntityList(component.{name}),"); + handleStateSetters.Append($@" + component.{name} = EnsureEntityList<{componentName}>(state.{name}, uid);"); + break; default: stateFields.Append($@" public {typeDisplayStr} {name} = default!;"); - if (attribute.ConstructorArguments[0].Value is bool val && val) + if (IsCloneType(type)) { // get first ctor arg of the field attribute, which determines whether the field should be cloned // (like if its a dict or list) @@ -163,7 +192,9 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName {name} = component.{name},"); handleStateSetters.Append($@" - if (state.{name} != null) + if (state.{name} == null) + component.{name} = null; + else component.{name} = new(state.{name});"); } else @@ -175,26 +206,6 @@ private static string GenerateSource(in GeneratorExecutionContext context, IName component.{name} = state.{name};"); } - break; - case GlobalEntityUidSetName: - stateFields.Append($@" - public {GlobalNetEntityUidSetName} {name} = default!;"); - - getStateInit.Append($@" - {name} = GetNetEntitySet(component.{name}),"); - handleStateSetters.Append($@" - component.{name} = EnsureEntitySet<{componentName}>(state.{name}, uid);"); - - break; - case GlobalEntityUidListName: - stateFields.Append($@" - public {GlobalNetEntityUidListName} {name} = default!;"); - - getStateInit.Append($@" - {name} = GetNetEntityList(component.{name}),"); - handleStateSetters.Append($@" - component.{name} = EnsureEntityList<{componentName}>(state.{name}, uid);"); - break; } } @@ -353,5 +364,20 @@ public void Initialize(GeneratorInitializationContext context) } context.RegisterForSyntaxNotifications(() => new NameReferenceSyntaxReceiver()); } + + private static bool IsCloneType(ITypeSymbol type) + { + if (type is not INamedTypeSymbol named || !named.IsGenericType) + { + return false; + } + + var constructed = named.ConstructedFrom.ToDisplayString(FullyQualifiedFormat); + return constructed switch + { + GlobalDictionaryName or GlobalHashSetName or GlobalListName => true, + _ => false + }; + } } } diff --git a/Robust.Shared.CompNetworkGenerator/Robust.Shared.CompNetworkGenerator.csproj b/Robust.Shared.CompNetworkGenerator/Robust.Shared.CompNetworkGenerator.csproj index 878ba53d5c6..3372817dd55 100644 --- a/Robust.Shared.CompNetworkGenerator/Robust.Shared.CompNetworkGenerator.csproj +++ b/Robust.Shared.CompNetworkGenerator/Robust.Shared.CompNetworkGenerator.csproj @@ -2,6 +2,7 @@ netstandard2.0 + 9 diff --git a/Robust.Shared/Analyzers/ComponentNetworkGeneratorAuxiliary.cs b/Robust.Shared/Analyzers/ComponentNetworkGeneratorAuxiliary.cs index 4018bdd8cae..dd97f78d6f2 100644 --- a/Robust.Shared/Analyzers/ComponentNetworkGeneratorAuxiliary.cs +++ b/Robust.Shared/Analyzers/ComponentNetworkGeneratorAuxiliary.cs @@ -32,20 +32,6 @@ public AutoGenerateComponentStateAttribute(bool raiseAfterAutoHandleState = fals [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)] public sealed class AutoNetworkedFieldAttribute : Attribute { - /// - /// Determines whether the data should be wrapped in a new() when setting in get/handlestate - /// e.g. for cloning collections like dictionaries or hashsets which is sometimes necessary. - /// - /// - /// This should only be true if the type actually has a constructor that takes in itself. - /// - [UsedImplicitly] - public bool CloneData; - - public AutoNetworkedFieldAttribute(bool cloneData=false) - { - CloneData = cloneData; - } } ///