From 8f0b2be7eec8d68f27e7e5b483f56118d19248a2 Mon Sep 17 00:00:00 2001 From: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com> Date: Wed, 18 Dec 2024 00:52:44 +0000 Subject: [PATCH 1/5] move atom name/desc handling to always defer to appearance --- .../DMProject/Tests/Image/appearance.dm | 10 +++++++ .../DebugWindows/IconDebugWindow.xaml.cs | 1 + OpenDreamRuntime/AtomManager.cs | 9 +++++++ OpenDreamRuntime/Objects/DreamObject.cs | 1 + .../Objects/Types/DreamObjectAtom.cs | 26 +++++-------------- .../Objects/Types/DreamObjectMovable.cs | 2 +- OpenDreamShared/Dream/IconAppearance.cs | 4 +++ .../Network/Messages/MsgAllAppearances.cs | 9 +++++++ 8 files changed, 41 insertions(+), 21 deletions(-) create mode 100644 Content.Tests/DMProject/Tests/Image/appearance.dm diff --git a/Content.Tests/DMProject/Tests/Image/appearance.dm b/Content.Tests/DMProject/Tests/Image/appearance.dm new file mode 100644 index 0000000000..3fa7925281 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Image/appearance.dm @@ -0,0 +1,10 @@ +/obj/thingtocopy + name = "hello" + desc = "this is a thing" + +/proc/RunTest() + var/obj/thingtocopy/T = new() + var/obj/otherthing = new() + otherthing.appearance = T.appearance + ASSERT(otherthing.name == T.name) + ASSERT(otherthing.desc == T.desc) \ No newline at end of file diff --git a/OpenDreamClient/Interface/DebugWindows/IconDebugWindow.xaml.cs b/OpenDreamClient/Interface/DebugWindows/IconDebugWindow.xaml.cs index 6d8d2d273c..ef10de75f9 100644 --- a/OpenDreamClient/Interface/DebugWindows/IconDebugWindow.xaml.cs +++ b/OpenDreamClient/Interface/DebugWindows/IconDebugWindow.xaml.cs @@ -45,6 +45,7 @@ private void Update() { // Would be nice if we could use ViewVariables instead, but I couldn't find a nice way to do that // Would be especially nice if we could use VV to make these editable AddPropertyIfNotDefault("Name", appearance.Name, IconAppearance.Default.Name); + AddPropertyIfNotDefault("Desc", appearance.Desc, IconAppearance.Default.Desc); AddPropertyIfNotDefault("Icon State", appearance.IconState, IconAppearance.Default.IconState); AddPropertyIfNotDefault("Direction", appearance.Direction, IconAppearance.Default.Direction); AddPropertyIfNotDefault("Inherits Direction", appearance.InheritsDirection, IconAppearance.Default.InheritsDirection); diff --git a/OpenDreamRuntime/AtomManager.cs b/OpenDreamRuntime/AtomManager.cs index 32d2bb5ba9..a0c3145639 100644 --- a/OpenDreamRuntime/AtomManager.cs +++ b/OpenDreamRuntime/AtomManager.cs @@ -209,6 +209,7 @@ public void DeleteMovableEntity(DreamObjectMovable movable) { public bool IsValidAppearanceVar(string name) { switch (name) { case "name": + case "desc": case "icon": case "icon_state": case "dir": @@ -248,6 +249,10 @@ public void SetAppearanceVar(IconAppearance appearance, string varName, DreamVal value.TryGetValueAsString(out var name); appearance.Name = name ?? string.Empty; break; + case "desc": + value.TryGetValueAsString(out var desc); + appearance.Desc = desc ?? string.Empty; + break; case "icon": if (_resourceManager.TryLoadIcon(value, out var icon)) { appearance.Icon = icon.Id; @@ -387,6 +392,8 @@ public DreamValue GetAppearanceVar(IconAppearance appearance, string varName) { switch (varName) { case "name": return new(appearance.Name); + case "desc": + return new(appearance.Desc); case "icon": if (appearance.Icon == null) return DreamValue.Null; @@ -589,6 +596,7 @@ public IconAppearance GetAppearanceFromDefinition(DreamObjectDefinition def) { return appearance; def.TryGetVariable("name", out var nameVar); + def.TryGetVariable("desc", out var descVar); def.TryGetVariable("icon", out var iconVar); def.TryGetVariable("icon_state", out var stateVar); def.TryGetVariable("color", out var colorVar); @@ -609,6 +617,7 @@ public IconAppearance GetAppearanceFromDefinition(DreamObjectDefinition def) { appearance = new IconAppearance(); SetAppearanceVar(appearance, "name", nameVar); + SetAppearanceVar(appearance, "desc", descVar); SetAppearanceVar(appearance, "icon", iconVar); SetAppearanceVar(appearance, "icon_state", stateVar); SetAppearanceVar(appearance, "color", colorVar); diff --git a/OpenDreamRuntime/Objects/DreamObject.cs b/OpenDreamRuntime/Objects/DreamObject.cs index cfa73be66b..99a390347a 100644 --- a/OpenDreamRuntime/Objects/DreamObject.cs +++ b/OpenDreamRuntime/Objects/DreamObject.cs @@ -13,6 +13,7 @@ using Robust.Shared.Map; using Robust.Shared.Serialization.Manager; using Robust.Shared.Utility; +using System.ComponentModel; namespace OpenDreamRuntime.Objects { [Virtual] diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs index df8754eb48..9001e81656 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs @@ -5,7 +5,6 @@ namespace OpenDreamRuntime.Objects.Types; [Virtual] public class DreamObjectAtom : DreamObject { - public string? Name; public string? Desc; public readonly DreamOverlaysList Overlays; public readonly DreamOverlaysList Underlays; @@ -22,11 +21,6 @@ public DreamObjectAtom(DreamObjectDefinition objectDefinition) : base(objectDefi AtomManager.AddAtom(this); } - public override void Initialize(DreamProcArguments args) { - ObjectDefinition.Variables["name"].TryGetValueAsString(out Name); - ObjectDefinition.Variables["desc"].TryGetValueAsString(out Desc); - } - protected override void HandleDeletion(bool possiblyThreaded) { // SAFETY: RemoveAtom is not threadsafe. if (possiblyThreaded) { @@ -38,6 +32,12 @@ protected override void HandleDeletion(bool possiblyThreaded) { base.HandleDeletion(possiblyThreaded); } + public string GetDesc() { + if (!TryGetVariable("desc", out DreamValue descVar) || !descVar.TryGetValueAsString(out string? desc)) + return ObjectDefinition.Type.ToString(); + + return desc; + } protected override bool TryGetVar(string varName, out DreamValue value) { switch (varName) { @@ -50,13 +50,6 @@ protected override bool TryGetVar(string varName, out DreamValue value) { case "loc": value = DreamValue.Null; return true; - - case "name": - value = (Name != null) ? new(Name) : DreamValue.Null; - return true; - case "desc": - value = (Desc != null) ? new(Desc) : DreamValue.Null; - return true; case "appearance": var appearanceCopy = new IconAppearance(AtomManager.MustGetAppearance(this)!); @@ -102,13 +95,6 @@ protected override void SetVar(string varName, DreamValue value) { case "z": case "loc": break; - - case "name": - value.TryGetValueAsString(out Name); - break; - case "desc": - value.TryGetValueAsString(out Desc); - break; case "appearance": if (!AtomManager.TryCreateAppearanceFrom(value, out var newAppearance)) return; // Ignore attempts to set an invalid appearance diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs b/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs index 9b38720942..44479a0d36 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs @@ -49,7 +49,7 @@ public override void Initialize(DreamProcArguments args) { if (EntityManager.TryGetComponent(Entity, out MetaDataComponent? metaData)) { MetaDataSystem?.SetEntityName(Entity, GetDisplayName(), metaData); - MetaDataSystem?.SetEntityDescription(Entity, Desc ?? string.Empty, metaData); + MetaDataSystem?.SetEntityDescription(Entity, GetDesc(), metaData); } args.GetArgument(0).TryGetValueAsDreamObject(out var loc); diff --git a/OpenDreamShared/Dream/IconAppearance.cs b/OpenDreamShared/Dream/IconAppearance.cs index 78e38af359..31c7db1878 100644 --- a/OpenDreamShared/Dream/IconAppearance.cs +++ b/OpenDreamShared/Dream/IconAppearance.cs @@ -24,6 +24,7 @@ public sealed class IconAppearance : IEquatable { public static readonly IconAppearance Default = new(); [ViewVariables] public string Name = string.Empty; + [ViewVariables] public string Desc = string.Empty; [ViewVariables] public int? Icon; [ViewVariables] public string? IconState; [ViewVariables] public AtomDirection Direction = AtomDirection.South; @@ -82,6 +83,7 @@ public IconAppearance() { public IconAppearance(IconAppearance appearance) { Name = appearance.Name; + Desc = appearance.Desc; Icon = appearance.Icon; IconState = appearance.IconState; Direction = appearance.Direction; @@ -119,6 +121,7 @@ public bool Equals(IconAppearance? appearance) { if (appearance == null) return false; if (appearance.Name != Name) return false; + if (appearance.Desc != Desc) return false; if (appearance.Icon != Icon) return false; if (appearance.IconState != IconState) return false; if (appearance.Direction != Direction) return false; @@ -207,6 +210,7 @@ public override int GetHashCode() { HashCode hashCode = new HashCode(); hashCode.Add(Name); + hashCode.Add(Desc); hashCode.Add(Icon); hashCode.Add(IconState); hashCode.Add(Direction); diff --git a/OpenDreamShared/Network/Messages/MsgAllAppearances.cs b/OpenDreamShared/Network/Messages/MsgAllAppearances.cs index 4377d00aa6..06c50ec76d 100644 --- a/OpenDreamShared/Network/Messages/MsgAllAppearances.cs +++ b/OpenDreamShared/Network/Messages/MsgAllAppearances.cs @@ -14,6 +14,7 @@ public sealed class MsgAllAppearances(Dictionary allAppeara private enum Property : byte { Name, + Desc, Icon, IconState, Direction, @@ -66,6 +67,9 @@ public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer case Property.Name: appearance.Name = buffer.ReadString(); break; + case Property.Desc: + appearance.Desc = buffer.ReadString(); + break; case Property.Id: appearanceId = buffer.ReadVariableInt32(); break; @@ -230,6 +234,11 @@ public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer buffer.Write(appearance.Name); } + if (appearance.Desc != IconAppearance.Default.Desc) { + buffer.Write((byte)Property.Desc); + buffer.Write(appearance.Desc); + } + if (appearance.Icon != null) { buffer.Write((byte)Property.Icon); buffer.WriteVariableInt32(appearance.Icon.Value); From d8059b8aa47aa51045c86b45fa3e5b0fa351c9cd Mon Sep 17 00:00:00 2001 From: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com> Date: Wed, 18 Dec 2024 16:41:50 +0000 Subject: [PATCH 2/5] amy review! --- OpenDreamRuntime/Objects/DreamObject.cs | 1 - OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/OpenDreamRuntime/Objects/DreamObject.cs b/OpenDreamRuntime/Objects/DreamObject.cs index 99a390347a..cfa73be66b 100644 --- a/OpenDreamRuntime/Objects/DreamObject.cs +++ b/OpenDreamRuntime/Objects/DreamObject.cs @@ -13,7 +13,6 @@ using Robust.Shared.Map; using Robust.Shared.Serialization.Manager; using Robust.Shared.Utility; -using System.ComponentModel; namespace OpenDreamRuntime.Objects { [Virtual] diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs index 9001e81656..9256f74f64 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs @@ -5,7 +5,6 @@ namespace OpenDreamRuntime.Objects.Types; [Virtual] public class DreamObjectAtom : DreamObject { - public string? Desc; public readonly DreamOverlaysList Overlays; public readonly DreamOverlaysList Underlays; public readonly DreamVisContentsList VisContents; From b82d31c99f6c085ae0ab7c222ec884c0c8cc715f Mon Sep 17 00:00:00 2001 From: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com> Date: Wed, 18 Dec 2024 21:09:07 +0000 Subject: [PATCH 3/5] rename GetDesc + remove toString --- OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs | 5 +++-- OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs index 9256f74f64..dff9fb736d 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs @@ -31,9 +31,10 @@ protected override void HandleDeletion(bool possiblyThreaded) { base.HandleDeletion(possiblyThreaded); } - public string GetDesc() { + + public string GetRTEntityDesc() { if (!TryGetVariable("desc", out DreamValue descVar) || !descVar.TryGetValueAsString(out string? desc)) - return ObjectDefinition.Type.ToString(); + return ObjectDefinition.Type; return desc; } diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs b/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs index 44479a0d36..582d8647a2 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs @@ -49,7 +49,7 @@ public override void Initialize(DreamProcArguments args) { if (EntityManager.TryGetComponent(Entity, out MetaDataComponent? metaData)) { MetaDataSystem?.SetEntityName(Entity, GetDisplayName(), metaData); - MetaDataSystem?.SetEntityDescription(Entity, GetDesc(), metaData); + MetaDataSystem?.SetEntityDescription(Entity, GetRTEntityDesc(), metaData); } args.GetArgument(0).TryGetValueAsDreamObject(out var loc); From 6107459fd79769a51d6364c3d86e079885c643ea Mon Sep 17 00:00:00 2001 From: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com> Date: Wed, 18 Dec 2024 21:12:52 +0000 Subject: [PATCH 4/5] make desc nullable --- OpenDreamRuntime/AtomManager.cs | 4 +++- OpenDreamShared/Dream/IconAppearance.cs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/OpenDreamRuntime/AtomManager.cs b/OpenDreamRuntime/AtomManager.cs index a0c3145639..b74266abd0 100644 --- a/OpenDreamRuntime/AtomManager.cs +++ b/OpenDreamRuntime/AtomManager.cs @@ -251,7 +251,7 @@ public void SetAppearanceVar(IconAppearance appearance, string varName, DreamVal break; case "desc": value.TryGetValueAsString(out var desc); - appearance.Desc = desc ?? string.Empty; + appearance.Desc = desc; break; case "icon": if (_resourceManager.TryLoadIcon(value, out var icon)) { @@ -393,6 +393,8 @@ public DreamValue GetAppearanceVar(IconAppearance appearance, string varName) { case "name": return new(appearance.Name); case "desc": + if (appearance.Desc == null) + return DreamValue.Null; return new(appearance.Desc); case "icon": if (appearance.Icon == null) diff --git a/OpenDreamShared/Dream/IconAppearance.cs b/OpenDreamShared/Dream/IconAppearance.cs index 31c7db1878..633f098d7f 100644 --- a/OpenDreamShared/Dream/IconAppearance.cs +++ b/OpenDreamShared/Dream/IconAppearance.cs @@ -24,7 +24,7 @@ public sealed class IconAppearance : IEquatable { public static readonly IconAppearance Default = new(); [ViewVariables] public string Name = string.Empty; - [ViewVariables] public string Desc = string.Empty; + [ViewVariables] public string? Desc = string.Empty; [ViewVariables] public int? Icon; [ViewVariables] public string? IconState; [ViewVariables] public AtomDirection Direction = AtomDirection.South; From 1a362a11ebe78be7267ff7c0a1742d44740278a2 Mon Sep 17 00:00:00 2001 From: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com> Date: Wed, 18 Dec 2024 21:19:39 +0000 Subject: [PATCH 5/5] move test to integration --- .../DMProject/Tests/atom_appearance.dm | 2 +- Content.IntegrationTests/DMProject/code.dm | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) rename Content.Tests/DMProject/Tests/Image/appearance.dm => Content.IntegrationTests/DMProject/Tests/atom_appearance.dm (91%) diff --git a/Content.Tests/DMProject/Tests/Image/appearance.dm b/Content.IntegrationTests/DMProject/Tests/atom_appearance.dm similarity index 91% rename from Content.Tests/DMProject/Tests/Image/appearance.dm rename to Content.IntegrationTests/DMProject/Tests/atom_appearance.dm index 3fa7925281..4b07dda7e3 100644 --- a/Content.Tests/DMProject/Tests/Image/appearance.dm +++ b/Content.IntegrationTests/DMProject/Tests/atom_appearance.dm @@ -2,7 +2,7 @@ name = "hello" desc = "this is a thing" -/proc/RunTest() +/proc/test_appearance() var/obj/thingtocopy/T = new() var/obj/otherthing = new() otherthing.appearance = T.appearance diff --git a/Content.IntegrationTests/DMProject/code.dm b/Content.IntegrationTests/DMProject/code.dm index ebb99dd087..84eb93c685 100644 --- a/Content.IntegrationTests/DMProject/code.dm +++ b/Content.IntegrationTests/DMProject/code.dm @@ -30,4 +30,5 @@ test_color_matrix() test_range() test_verb_duplicate() + test_appearance() world.log << "IntegrationTests successful, /world/New() exiting..." \ No newline at end of file