From 64e6fe5da8d4b5244c98b61ed3731ccf54b3960c Mon Sep 17 00:00:00 2001 From: Guflly <145608489+Guflly@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:55:12 -0700 Subject: [PATCH] fix: Warn when sending empty gumps (#2563) ### Summary Generates a console warning when users receive an empty gump. This will help prevent client side leaks. --- .../Tests/Gumps/TestGumps/DynamicTestGump.cs | 2 + .../Tests/Gumps/TestGumps/EmptyTestGumps.cs | 40 +++++++++++++++++++ .../Tests/Gumps/TestGumps/LegacyTestGump.cs | 2 + .../Tests/Gumps/TestGumps/StaticTestGump.cs | 2 + .../Tests/Gumps/TestLayoutGumps.cs | 26 ++++++++++++ Projects/UOContent/Gumps/Base/BaseGump.cs | 8 ++++ Projects/UOContent/Gumps/Base/DynamicGump.cs | 1 + .../Gumps/Base/DynamicGumpBuilder.cs | 1 + .../UOContent/Gumps/Base/GumpLayoutBuilder.cs | 23 +++++++++++ Projects/UOContent/Gumps/Base/Legacy/Gump.cs | 5 +++ Projects/UOContent/Gumps/Base/StaticGump.cs | 3 ++ .../UOContent/Gumps/Base/StaticGumpBuilder.cs | 1 + 12 files changed, 114 insertions(+) create mode 100644 Projects/UOContent.Tests/Tests/Gumps/TestGumps/EmptyTestGumps.cs diff --git a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/DynamicTestGump.cs b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/DynamicTestGump.cs index 998e4c537..fd3dafc56 100644 --- a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/DynamicTestGump.cs +++ b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/DynamicTestGump.cs @@ -6,6 +6,8 @@ public class DynamicTestGump : DynamicGump { private readonly string _petName; + public bool HasVisualElementsForTest => HasVisualElements; + public DynamicTestGump(string petName) : base(50, 50) { _petName = petName; diff --git a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/EmptyTestGumps.cs b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/EmptyTestGumps.cs new file mode 100644 index 000000000..b99de1700 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/EmptyTestGumps.cs @@ -0,0 +1,40 @@ +using Server.Gumps; + +namespace Server.Tests.Gumps; + +public sealed class EmptyLegacyTestGump : Gump +{ + public bool HasVisualElementsForTest => HasVisualElements; + + public EmptyLegacyTestGump() : base(0, 0) + { + } +} + +public sealed class EmptyDynamicTestGump : DynamicGump +{ + public bool HasVisualElementsForTest => HasVisualElements; + + public EmptyDynamicTestGump() : base(0, 0) + { + } + + protected override void BuildLayout(ref DynamicGumpBuilder builder) + { + builder.AddPage(); + } +} + +public sealed class EmptyStaticTestGump : StaticGump +{ + public bool HasVisualElementsForTest => HasVisualElements; + + public EmptyStaticTestGump() : base(0, 0) + { + } + + protected override void BuildLayout(ref StaticGumpBuilder builder) + { + builder.SetNoClose(); + } +} diff --git a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/LegacyTestGump.cs b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/LegacyTestGump.cs index 668d7e8ae..e45514c38 100644 --- a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/LegacyTestGump.cs +++ b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/LegacyTestGump.cs @@ -4,6 +4,8 @@ namespace Server.Tests.Gumps; public sealed class LegacyTestGump : Gump { + public bool HasVisualElementsForTest => HasVisualElements; + public LegacyTestGump(string petName) : base(50, 50) { Serial = (Serial)0x123; diff --git a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/StaticTestGump.cs b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/StaticTestGump.cs index ea67720f5..66d80c963 100644 --- a/Projects/UOContent.Tests/Tests/Gumps/TestGumps/StaticTestGump.cs +++ b/Projects/UOContent.Tests/Tests/Gumps/TestGumps/StaticTestGump.cs @@ -4,6 +4,8 @@ namespace Server.Tests.Gumps; public class StaticTestGump : StaticGump { + public bool HasVisualElementsForTest => HasVisualElements; + public StaticTestGump() : base(50, 50) { Serial = (Serial)0x123; diff --git a/Projects/UOContent.Tests/Tests/Gumps/TestLayoutGumps.cs b/Projects/UOContent.Tests/Tests/Gumps/TestLayoutGumps.cs index 7cd8e67d1..804408fc7 100644 --- a/Projects/UOContent.Tests/Tests/Gumps/TestLayoutGumps.cs +++ b/Projects/UOContent.Tests/Tests/Gumps/TestLayoutGumps.cs @@ -73,6 +73,32 @@ public class TestLayoutGumps AssertThat.Equal(writer.Span, packet); } + [Fact] + public void TestEmptyGumpsHaveNoVisualElements() + { + Assert.False(Compile(new EmptyLegacyTestGump()).HasVisualElementsForTest); + Assert.False(Compile(new EmptyDynamicTestGump()).HasVisualElementsForTest); + Assert.False(Compile(new EmptyStaticTestGump()).HasVisualElementsForTest); + Assert.False(Compile(new EmptyStaticTestGump()).HasVisualElementsForTest); + } + + [Fact] + public void TestVisibleGumpsHaveVisualElements() + { + Assert.True(Compile(new LegacyTestGump("Test")).HasVisualElementsForTest); + Assert.True(Compile(new DynamicTestGump("Test")).HasVisualElementsForTest); + Assert.True(Compile(new StaticTestGump()).HasVisualElementsForTest); + Assert.True(Compile(new StaticTestGump()).HasVisualElementsForTest); + } + + private static T Compile(T gump) where T : BaseGump + { + var buffer = GC.AllocateUninitializedArray(512); + var writer = new SpanWriter(buffer); + gump.Compile(ref writer); + return gump; + } + private static void InternalTestStaticGump(ReadOnlySpan expectedLayout, StaticGump staticGump, string[] strings) where T : StaticGump { diff --git a/Projects/UOContent/Gumps/Base/BaseGump.cs b/Projects/UOContent/Gumps/Base/BaseGump.cs index 6d844735d..05770608c 100644 --- a/Projects/UOContent/Gumps/Base/BaseGump.cs +++ b/Projects/UOContent/Gumps/Base/BaseGump.cs @@ -13,6 +13,7 @@ * along with this program. If not, see . * *************************************************************************/ +using Server.Logging; using Server.Network; using System; using System.Buffers; @@ -23,10 +24,12 @@ namespace Server.Gumps; public abstract class BaseGump { private static readonly byte[] _packetBuffer = GC.AllocateUninitializedArray(0x10000); + private static readonly ILogger _logger = LogFactory.GetLogger(typeof(BaseGump)); private static Serial nextSerial = (Serial)1; public int TypeID { get; protected set; } public Serial Serial { get; protected set; } + protected bool HasVisualElements { get; set; } public abstract int Switches { get; } public abstract int TextEntries { get; } @@ -56,6 +59,11 @@ public abstract class BaseGump var writer = new SpanWriter(_packetBuffer); Compile(ref writer); + if (!HasVisualElements) + { + _logger.Warning("Sending empty gump {GumpType}", GetType().FullName); + } + ns.Send(writer.Span); writer.Dispose(); diff --git a/Projects/UOContent/Gumps/Base/DynamicGump.cs b/Projects/UOContent/Gumps/Base/DynamicGump.cs index f1b3c064a..de894da68 100644 --- a/Projects/UOContent/Gumps/Base/DynamicGump.cs +++ b/Projects/UOContent/Gumps/Base/DynamicGump.cs @@ -47,6 +47,7 @@ public abstract class DynamicGump : BaseGump BuildLayout(ref gumpBuilder); gumpBuilder.FinalizeLayout(); + HasVisualElements = gumpBuilder.HasVisualElements; _switches = gumpBuilder.Switches; _textEntries = gumpBuilder.TextEntries; diff --git a/Projects/UOContent/Gumps/Base/DynamicGumpBuilder.cs b/Projects/UOContent/Gumps/Base/DynamicGumpBuilder.cs index e98c8c6df..9238e6e04 100644 --- a/Projects/UOContent/Gumps/Base/DynamicGumpBuilder.cs +++ b/Projects/UOContent/Gumps/Base/DynamicGumpBuilder.cs @@ -36,6 +36,7 @@ public ref struct DynamicGumpBuilder public int Switches => _gumpBuilder._switches; public int TextEntries => _gumpBuilder._textEntries; + internal bool HasVisualElements => _gumpBuilder._hasVisualElements; [MethodImpl(MethodImplOptions.AggressiveInlining)] public DynamicGumpBuilder() diff --git a/Projects/UOContent/Gumps/Base/GumpLayoutBuilder.cs b/Projects/UOContent/Gumps/Base/GumpLayoutBuilder.cs index 7f0ca5dff..4bd5c7d8f 100644 --- a/Projects/UOContent/Gumps/Base/GumpLayoutBuilder.cs +++ b/Projects/UOContent/Gumps/Base/GumpLayoutBuilder.cs @@ -30,6 +30,7 @@ public ref struct GumpLayoutBuilder internal GumpFlags _flags; internal int _switches; internal int _textEntries; + internal bool _hasVisualElements; internal Span LayoutData => _layoutBuffer.AsSpan(0, _bytesWritten); @@ -95,6 +96,7 @@ public ref struct GumpLayoutBuilder public void AddBackground(int x, int y, int width, int height, int gumpId) { + _hasVisualElements = true; GrowIfNeeded(9 + 9 + 45); WriteStart("resizepic"u8); WriteValue(x); @@ -109,6 +111,7 @@ public ref struct GumpLayoutBuilder int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type = GumpButtonType.Reply, int param = 0 ) { + _hasVisualElements = true; GrowIfNeeded(11 + 6 + 54 + 2); WriteStart("button"u8); WriteValue(x); @@ -123,6 +126,7 @@ public ref struct GumpLayoutBuilder public void AddCheckbox(int x, int y, int inactiveId, int activeId, bool selected, int switchId) { + _hasVisualElements = true; GrowIfNeeded(10 + 8 + 45 + 2); WriteStart("checkbox"u8); WriteValue(x); @@ -154,6 +158,7 @@ public ref struct GumpLayoutBuilder public int AddHtmlPlaceholder(int x, int y, int width, int height, bool background = false, bool scrollbar = false) { + _hasVisualElements = true; GrowIfNeeded(11 + 8 + 36 + 10); WriteStart("htmlgump"u8); WriteValue(x); @@ -172,6 +177,7 @@ public ref struct GumpLayoutBuilder public void AddHtml(int x, int y, int width, int height, int text, bool background = false, bool scrollbar = false) { + _hasVisualElements = true; GrowIfNeeded(11 + 8 + 45 + 4); WriteStart("htmlgump"u8); WriteValue(x); @@ -188,6 +194,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int number, bool background = false, bool scrollbar = false ) { + _hasVisualElements = true; GrowIfNeeded(11 + 11 + 45 + 4); WriteStart("xmfhtmlgump"u8); WriteValue(x); @@ -204,6 +211,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int number, int color, bool background = false, bool scrollbar = false ) { + _hasVisualElements = true; GrowIfNeeded(12 + 16 + 45 + 5 + 4); WriteStart("xmfhtmlgumpcolor"u8); WriteValue(x); @@ -220,6 +228,7 @@ public ref struct GumpLayoutBuilder public void AddHtmlLocalized(int x, int y, int width, int height, int number, ReadOnlySpan args, int color, bool background = false, bool scrollbar = false) { + _hasVisualElements = true; GrowIfNeeded(12 + 10 + 45 + 5 + 4 + (args.Length > 0 ? 3 + args.Length : 0)); WriteStart("xmfhtmltok"u8); WriteValue(x); @@ -254,6 +263,7 @@ public ref struct GumpLayoutBuilder public void AddImage(int x, int y, int gumpId, int hue = 0, ReadOnlySpan cls = default) { + _hasVisualElements = true; GrowIfNeeded(7 + 7 + 36 + (hue != 0 ? 14 : 0) + (cls.Length > 0 ? 7 + cls.Length : 0)); WriteStart("gumppic"u8); WriteValue(x); @@ -294,6 +304,7 @@ public ref struct GumpLayoutBuilder public void AddImageTiledButton(int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type, int param, int itemId, int hue, int width, int height, int localizedTooltip = -1) { + _hasVisualElements = true; GrowIfNeeded(15 + 13 + 90 + 2); WriteStart("buttontileart"u8); WriteValue(x); @@ -319,6 +330,7 @@ public ref struct GumpLayoutBuilder public void AddImageTiled(int x, int y, int width, int height, int gumpId) { + _hasVisualElements = true; GrowIfNeeded(9 + 12 + 45); WriteStart("gumppictiled"u8); WriteValue(x); @@ -331,6 +343,7 @@ public ref struct GumpLayoutBuilder public void AddItem(int x, int y, int itemId, int hue = 0) { + _hasVisualElements = true; GrowIfNeeded(7 + 36 + (hue != 0 ? 20 : 7)); WriteStart(hue == 0 ? "tilepic"u8 : "tilepichue"u8); WriteValue(x); @@ -355,6 +368,7 @@ public ref struct GumpLayoutBuilder public int AddLabelPlaceholder(int x, int y, int hue) { + _hasVisualElements = true; GrowIfNeeded(8 + 4 + 27 + 6); WriteStart("text"u8); WriteValue(x); @@ -368,6 +382,7 @@ public ref struct GumpLayoutBuilder public void AddLabel(int x, int y, int hue, int text) { + _hasVisualElements = true; GrowIfNeeded(8 + 4 + 36 + 6); WriteStart("text"u8); WriteValue(x); @@ -379,6 +394,7 @@ public ref struct GumpLayoutBuilder public int AddLabelCroppedPlaceholder(int x, int y, int width, int height, int hue) { + _hasVisualElements = true; GrowIfNeeded(10 + 11 + 45 + 6); WriteStart("croppedtext"u8); WriteValue(x); @@ -394,6 +410,7 @@ public ref struct GumpLayoutBuilder public void AddLabelCropped(int x, int y, int width, int height, int hue, int text) { + _hasVisualElements = true; GrowIfNeeded(10 + 11 + 54 + 6); WriteStart("croppedtext"u8); WriteValue(x); @@ -430,6 +447,7 @@ public ref struct GumpLayoutBuilder public void AddRadio(int x, int y, int inactiveId, int activeId, bool selected, int switchId) { + _hasVisualElements = true; GrowIfNeeded(10 + 5 + 45 + 2); WriteStart("radio"u8); WriteValue(x); @@ -445,6 +463,7 @@ public ref struct GumpLayoutBuilder public void AddSpriteImage(int x, int y, int gumpId, int width, int height, int sx, int sy) { + _hasVisualElements = true; GrowIfNeeded(11 + 8 + 63); WriteStart("picinpic"u8); WriteValue(x); @@ -461,6 +480,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int hue, int entryId ) { + _hasVisualElements = true; GrowIfNeeded(11 + 9 + 54 + 6); WriteStart("textentry"u8); WriteValue(x); @@ -481,6 +501,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int hue, int entryId, int initialText ) { + _hasVisualElements = true; GrowIfNeeded(11 + 9 + 63 + 6); WriteStart("textentry"u8); WriteValue(x); @@ -499,6 +520,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int hue, int entryId, int size = 0 ) { + _hasVisualElements = true; GrowIfNeeded(12 + 16 + 63 + 6); WriteStart("textentrylimited"u8); WriteValue(x); @@ -520,6 +542,7 @@ public ref struct GumpLayoutBuilder int x, int y, int width, int height, int hue, int entryId, int initialText, int size = 0 ) { + _hasVisualElements = true; GrowIfNeeded(12 + 16 + 72); WriteStart("textentrylimited"u8); WriteValue(x); diff --git a/Projects/UOContent/Gumps/Base/Legacy/Gump.cs b/Projects/UOContent/Gumps/Base/Legacy/Gump.cs index 5cfb1f14a..3be1ddebf 100644 --- a/Projects/UOContent/Gumps/Base/Legacy/Gump.cs +++ b/Projects/UOContent/Gumps/Base/Legacy/Gump.cs @@ -249,6 +249,7 @@ public class Gump : BaseGump { _textEntries = 0; _switches = 0; + HasVisualElements = false; var layoutWriter = new SpanWriter(_layoutBuffer); @@ -274,6 +275,7 @@ public class Gump : BaseGump foreach (var entry in Entries) { + HasVisualElements |= IsVisualEntry(entry); entry.AppendTo(ref layoutWriter, _stringsList, ref _textEntries, ref _switches); } @@ -311,6 +313,9 @@ public class Gump : BaseGump } } + private static bool IsVisualEntry(GumpEntry entry) => + entry is not (GumpAlphaRegion or GumpECHandleInput or GumpGroup or GumpItemProperty or GumpMasterGump or GumpPage or GumpTooltip); + protected void Reset() { _switches = 0; diff --git a/Projects/UOContent/Gumps/Base/StaticGump.cs b/Projects/UOContent/Gumps/Base/StaticGump.cs index a10491618..0ab2df5e0 100644 --- a/Projects/UOContent/Gumps/Base/StaticGump.cs +++ b/Projects/UOContent/Gumps/Base/StaticGump.cs @@ -30,6 +30,7 @@ public abstract class StaticGump : BaseGump where TSelf : StaticGump : BaseGump where TSelf : StaticGump : BaseGump where TSelf : StaticGump _gumpBuilder._switches; public int TextEntries => _gumpBuilder._textEntries; + internal bool HasVisualElements => _gumpBuilder._hasVisualElements; [MethodImpl(MethodImplOptions.AggressiveInlining)] public StaticGumpBuilder()