Minor improvements to printing in VV (#1041)

Robust.Client/ViewVariables/Editors/ViewVariablesPropertyEditorReference.cs

@@ -19,13 +19,7 @@ namespace Robust.Client.ViewVariables.Editors
             // NOTE: value is NOT always the actual object.
             // Only thing we can really rely on is that ToString works out correctly.
             // This is because of reference tokens, but due to simplicity the object ref is still passed.
-
-            var toString = value.ToString();
-            if (value.GetType().FullName == toString)
-            {
-                toString = TypeAbbreviation.Abbreviate(toString);
-            }
-
+            var toString = PrettyPrint.PrintUserFacing(value);
             var button = new Button
             {
                 Text = $"Ref: {toString}",

Robust.Client/ViewVariables/Instances/ViewVariablesInstanceEntity.cs

@@ -68,8 +68,8 @@ namespace Robust.Client.ViewVariables.Instances
             // Handle top bar displaying type and ToString().
             {
                 Control top;
-                var stringified = obj.ToString();
-                if (type.FullName != stringified)
+                var stringified = PrettyPrint.PrintUserFacingWithType(obj, out var typeStringified);
+                if (typeStringified != "")
                 {
                     //var smallFont = new VectorFont(_resourceCache.GetResource<FontResource>("/Fonts/CALIBRI.TTF"), 10);
                     // Custom ToString() implementation.
@@ -77,7 +77,7 @@ namespace Robust.Client.ViewVariables.Instances
                     headBox.AddChild(new Label {Text = stringified, ClipText = true});
                     headBox.AddChild(new Label
                     {
-                        Text = TypeAbbreviation.Abbreviate(type.FullName),
+                        Text = typeStringified,
                     //    FontOverride = smallFont,
                         FontColorOverride = Color.DarkGray,
                         ClipText = true
@@ -86,7 +86,7 @@ namespace Robust.Client.ViewVariables.Instances
                 }
                 else
                 {
-                    top = new Label {Text = TypeAbbreviation.Abbreviate(stringified)};
+                    top = new Label {Text = stringified};
                 }
 
                 if (_entity.TryGetComponent(out ISpriteComponent sprite))
@@ -124,7 +124,7 @@ namespace Robust.Client.ViewVariables.Instances
             var componentList = _entity.GetAllComponents().OrderBy(c => c.GetType().ToString());
             foreach (var component in componentList)
             {
-                var button = new Button {Text = TypeAbbreviation.Abbreviate(component.GetType().ToString()), TextAlign = Label.AlignMode.Left};
+                var button = new Button {Text = TypeAbbreviation.Abbreviate(component.GetType()), TextAlign = Label.AlignMode.Left};
                 button.OnPressed += args => { ViewVariablesManager.OpenVV(component); };
                 clientComponents.AddChild(button);
             }

Robust.Client/ViewVariables/Instances/ViewVariablesInstanceObject.cs

@@ -27,12 +27,7 @@ namespace Robust.Client.ViewVariables.Instances
             Object = obj;
             var type = obj.GetType();
 
-            var title = obj.ToString();
-            var subtitle = TypeAbbreviation.Abbreviate(type.ToString());
-            if (title == obj.GetType().FullName) {
-                title = TypeAbbreviation.Abbreviate(title);
-                subtitle = ""; // This would just be the type again - not helpful
-            }
+            var title = PrettyPrint.PrintUserFacingWithType(obj, out var subtitle);
 
             _wrappingInit(window, title, subtitle);
             foreach (var trait in TraitsFor(ViewVariablesManager.TraitIdsFor(type)))

Robust.Client/ViewVariables/ViewVariablesInstance.cs

@@ -101,7 +101,7 @@ namespace Robust.Client.ViewVariables
                     Editable = access == VVAccess.ReadWrite,
                     Name = memberInfo.Name,
                     Type = memberType.AssemblyQualifiedName,
-                    TypePretty = TypeAbbreviation.Abbreviate(memberType.ToString()),
+                    TypePretty = TypeAbbreviation.Abbreviate(memberType),
                     Value = value
                 };
 

Robust.Server/ViewVariables/Traits/ViewVariablesTraitEntity.cs

@@ -24,7 +24,7 @@ namespace Robust.Server.ViewVariables.Traits
                 {
                     var type = component.GetType();
                     list.Add(new ViewVariablesBlobEntityComponents.Entry
-                        {Stringified = TypeAbbreviation.Abbreviate(type.ToString()), FullName = type.FullName});
+                        {Stringified = TypeAbbreviation.Abbreviate(type), FullName = type.FullName});
                 }
 
                 return new ViewVariablesBlobEntityComponents

Robust.Server/ViewVariables/Traits/ViewVariablesTraitMembers.cs

@@ -46,7 +46,7 @@ namespace Robust.Server.ViewVariables.Traits
                     Editable = attr.Access == VVAccess.ReadWrite,
                     Name = property.Name,
                     Type = property.PropertyType.AssemblyQualifiedName,
-                    TypePretty = TypeAbbreviation.Abbreviate(property.PropertyType.ToString()),
+                    TypePretty = TypeAbbreviation.Abbreviate(property.PropertyType),
                     Value = property.GetValue(Session.Object),
                     PropertyIndex = _members.Count
                 });
@@ -66,7 +66,7 @@ namespace Robust.Server.ViewVariables.Traits
                     Editable = attr.Access == VVAccess.ReadWrite,
                     Name = field.Name,
                     Type = field.FieldType.AssemblyQualifiedName,
-                    TypePretty = TypeAbbreviation.Abbreviate(field.FieldType.ToString()),
+                    TypePretty = TypeAbbreviation.Abbreviate(field.FieldType),
                     Value = field.GetValue(Session.Object),
                     PropertyIndex = _members.Count
                 });

Robust.Server/ViewVariables/ViewVariablesSession.cs

@@ -60,7 +60,7 @@ namespace Robust.Server.ViewVariables
                 return new ViewVariablesBlobMetadata
                 {
                     ObjectType = ObjectType.AssemblyQualifiedName,
-                    ObjectTypePretty = TypeAbbreviation.Abbreviate(ObjectType.ToString()),
+                    ObjectTypePretty = TypeAbbreviation.Abbreviate(ObjectType),
                     Stringified = Object.ToString(),
                     Traits = new List<object>(Host.TraitIdsFor(ObjectType))
                 };

Robust.Server/ViewVariables/ViewVariablesTrait.cs

@@ -94,14 +94,9 @@ namespace Robust.Server.ViewVariables
                 // TODO: More flexibility in which types can be sent here.
                 if (valType != typeof(string))
                 {
-                    var stringified = value.ToString();
-                    if (stringified == value.GetType().FullName)
-                    {
-                        stringified = TypeAbbreviation.Abbreviate(stringified);
-                    }
                     return new ViewVariablesBlobMembers.ReferenceToken
                     {
-                        Stringified = stringified
+                        Stringified = PrettyPrint.PrintUserFacing(value)
                     };
                 }
             }

Robust.Shared/Utility/PrettyPrint.cs

@@ -0,0 +1,67 @@
+using System;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+
+namespace Robust.Shared.Utility
+{
+    /// <summary>
+    ///     Utility class for producing debug menu representations.
+    /// </summary>
+    public static class PrettyPrint
+    {
+
+        /// <summary>
+        ///     Get the user-facing string representation of a value.
+        ///
+        ///     This is intended for menus where users are required to look at
+        ///     some kind of raw engine representation. It is not a substitute
+        ///     for a proper UI.
+        /// </summary>
+        /// <param name="value">The object to represent.</param>
+        /// <returns>A readable representation of the object.</returns>
+        public static string PrintUserFacing(object value)
+        {
+           return PrintUserFacingWithType(value, out _);
+        }
+
+        /// <summary>
+        ///     Get the user-facing string representation of a value, along with
+        ///     the representation of its type.
+        ///
+        ///     See <see cref='PrintUserFacing(object)'/> for usage details. This
+        ///     also returns a user-facing representation of the object's type in
+        ///     <paramref name="typeRep"/> if it is different to that of the object.
+        ///     If the object's <c>ToString()</c> implementation is the default
+        ///     one, then <paramref name="typeRep"/> will be <c>""</c>.
+        /// </summary>
+        /// <param name="value">The object to represent.</param>
+        /// <param name="typeRef">
+        ///   The representation of the object's type, if distinct from the
+        ///   returned value. Otherwise, <c>""</c>.
+        /// </param>
+        /// <returns>A readable representation of the object.</returns>
+        public static string PrintUserFacingWithType(object value, out string typeRep)
+        {
+            if (value == null) {
+                typeRep = string.Empty;
+                return "null";
+            }
+
+            string stringRep;
+            // Make best effort to guess whether or not this needs an abbreviated
+            // type representation - if the type doesn't overwrite the default
+            // `Object` `ToString`, then it will just print a type - so we instead
+            // print the abbreviated version. Otherwise let the type print whatever
+            // it wants
+            if (value.GetType().GetMethod("ToString", new Type[0], new ParameterModifier[0]).DeclaringType == typeof(Object)) {
+                stringRep = TypeAbbreviation.Abbreviate(value.GetType());
+                typeRep = string.Empty;
+            } else {
+                stringRep = value.ToString();
+                typeRep = TypeAbbreviation.Abbreviate(value.GetType());
+            }
+
+            return stringRep;
+        }
+    }
+}

Robust.Shared/Utility/TypeAbbreviation.cs

@@ -30,20 +30,52 @@ namespace Robust.Shared.Utility
         }
 
         /// <summary>
-        ///     Attempt to abbreviate a full type name into something shorter.
+        ///     Attempt to produce a shorter version of a type's full representation.
         /// </summary>
-        /// <param name="name">The type name to abbreviate.</param>
-        /// <returns>A shorter, but still unique, version of the passed type name.</returns>
-        public static string Abbreviate(ReadOnlySpan<char> name)
+        /// <param name="type">The type to abbreviate.</param>
+        /// <returns>A shorter representation of the passed type than given by `ToString()`.</returns>
+        public static string Abbreviate(Type type)
         {
             var sb = new StringBuilder();
 
-            Abbreviate(name, _abbreviations, sb);
+            // `Type.FullName` assembly-qualifies all type arguments, but we don't
+            // want them to be qualified - hence, this hack. We just take the name
+            // before the type arguments by ignoring characters after the generic
+            // argument number marker <c>`</c>.
+            AbbreviateName(type.FullName.Split('`')[0], _abbreviations, sb);
+
+            // Never null - this is just empty if the type is non-generic
+            var genericArgs = type.GetGenericArguments();
+            if (genericArgs.Length > 0) {
+                // Match Type's `ToString()` - start with the number of arguments
+                sb.Append("`").Append(genericArgs.Length).Append("[");
+                foreach (var genericArg in genericArgs) {
+                    AbbreviateName(genericArg.FullName, _abbreviations, sb);
+                }
+                sb.Append("]");
+            }
 
             return sb.ToString();
         }
 
-        private static void Abbreviate(ReadOnlySpan<char> name, Abbreviation[] abbreviations, StringBuilder output)
+        /// <summary>
+        ///     Attempt to abbreviate a full name into something shorter.
+        ///
+        ///     For types, use <see cref="Abbreviate(Type)"/> instead, since it
+        ///     correctly handles the more complex type logic.
+        /// </summary>
+        /// <param name="name">The name to abbreviate.</param>
+        /// <returns>A shorter, but still unique, version of the passed named.</returns>
+        public static string Abbreviate(string name)
+        {
+            var sb = new StringBuilder();
+
+            AbbreviateName(name, _abbreviations, sb);
+
+            return sb.ToString();
+        }
+
+        private static void AbbreviateName(ReadOnlySpan<char> name, Abbreviation[] abbreviations, StringBuilder output)
         {
             foreach (var abbr in abbreviations)
             {
@@ -58,7 +90,7 @@ namespace Robust.Shared.Utility
 
                 if (abbr.SubAbbreviations.Length != 0)
                 {
-                    Abbreviate(name, abbr.SubAbbreviations, output);
+                    AbbreviateName(name, abbr.SubAbbreviations, output);
                     // Return so nested call can handle appending final name.
                     return;
                 }

Robust.UnitTesting/Shared/Utility/PrettyPrint_Test.cs

@@ -0,0 +1,38 @@
+using System;
+using System.Collections.Generic;
+using NUnit.Framework;
+using Robust.Shared.Utility;
+
+namespace Robust.Shared.TestPrettyPrint
+{
+    public class Foo
+    {
+        override public string ToString() { return "ACustomFooRep"; }
+    }
+
+    public class Bar {}
+}
+
+namespace Robust.UnitTesting.Shared.Utility
+{
+
+
+    [TestFixture]
+    [Parallelizable(ParallelScope.Fixtures | ParallelScope.All)]
+    [TestOf(typeof(PrettyPrint))]
+    public class PrettyPrint_Test
+    {
+        private static IEnumerable<(object val, string expectedRep, string expectedTypeRep)> TestCases { get; } = new (object, string, string)[]
+        {
+            (new Robust.Shared.TestPrettyPrint.Foo(), "ACustomFooRep", "R.Sh.TestPrettyPrint.Foo"),
+            (new Robust.Shared.TestPrettyPrint.Bar(), "R.Sh.TestPrettyPrint.Bar", ""),
+        };
+
+        [Test]
+        public void Test([ValueSource(nameof(TestCases))] (object value, string expectedRep, string expectedTypeRep) data)
+        {
+            Assert.That(PrettyPrint.PrintUserFacingWithType(data.value, out var typeRep), Is.EqualTo(data.expectedRep));
+            Assert.That(typeRep, Is.EqualTo(data.expectedTypeRep));
+        }
+    }
+}

Robust.UnitTesting/Shared/Utility/TypeAbbreviations_Test.cs

@@ -1,7 +1,15 @@
+using System;
 using System.Collections.Generic;
 using NUnit.Framework;
 using Robust.Shared.Utility;
 
+namespace Robust.Shared.TestTypeAbbreviation
+{
+    public class Foo<T> {}
+
+    public class Bar {}
+}
+
 namespace Robust.UnitTesting.Shared.Utility
 {
     [TestFixture]
@@ -9,7 +17,7 @@ namespace Robust.UnitTesting.Shared.Utility
     [TestOf(typeof(TypeAbbreviation))]
     public class TypeAbbreviations_Test
     {
-        private static IEnumerable<(string name, string expected)> TestCases { get; } = new[]
+        private static IEnumerable<(string name, string expected)> NameTestCases { get; } = new[]
         {
             ("Robust.Shared.GameObjects.Foo", "R.Sh.GO.Foo"),
             ("Robust.Client.GameObjects.Foo", "R.C.GO.Foo"),
@@ -20,9 +28,24 @@ namespace Robust.UnitTesting.Shared.Utility
         };
 
         [Test]
-        public void Test([ValueSource(nameof(TestCases))] (string name, string expected) data)
+        public void Test([ValueSource(nameof(NameTestCases))] (string name, string expected) data)
         {
             Assert.That(TypeAbbreviation.Abbreviate(data.name), Is.EqualTo(data.expected));
         }
+
+
+        private static IEnumerable<(Type type, string expected)> TypeTestCases { get; } = new[]
+        {
+            ( typeof(Robust.Shared.TestTypeAbbreviation.Foo<Robust.Shared.TestTypeAbbreviation.Bar>)
+            , "R.Sh.TestTypeAbbreviation.Foo`1[R.Sh.TestTypeAbbreviation.Bar]"
+            ),
+            (typeof(Robust.Shared.TestTypeAbbreviation.Bar), "R.Sh.TestTypeAbbreviation.Bar"),
+        };
+
+        [Test]
+        public void Test([ValueSource(nameof(TypeTestCases))] (Type type, string expected) data)
+        {
+            Assert.That(TypeAbbreviation.Abbreviate(data.type), Is.EqualTo(data.expected));
+        }
     }
 }