Remove unneeded chrome yaml declarations

[dragunoff]

An oversight from #17663

These declarations are not needed because dropdowns and scrollbars fall back to the base name if there is no image defined for a given state and the state sprites in these cases are the same as the base.

[pchote]

The fallback behaviour requires additional lookups, so wouldn't it be better for performance to keep them?

[dragunoff]

The fallback behaviour requires additional lookups, so wouldn't it be better for performance to keep them?

What about mods that don't need/have art for all states (as is the case for d2k and actually for all core mods)? We should then define all states or figure out a way to do that without a performance hit.

[pchote]

Its not pretty, but maybe something like this would work?

diff --git a/OpenRA.Mods.Common/Widgets/DropDownButtonWidget.cs b/OpenRA.Mods.Common/Widgets/DropDownButtonWidget.cs
index 8c104e2fab..f1805a3a7e 100644
--- a/OpenRA.Mods.Common/Widgets/DropDownButtonWidget.cs
+++ b/OpenRA.Mods.Common/Widgets/DropDownButtonWidget.cs
@@ -29,18 +29,29 @@ public class DropDownButtonWidget : ButtonWidget
                Widget panel;
                MaskWidget fullscreenMask;
                Widget panelRoot;
+               readonly CachedTransform<(bool Disabled, bool Pressed, bool Hover), Sprite> getSeparatorImage;
 
                public string PanelRoot;
                public string SelectedItem;
 
+               Sprite GetSeperatorImageInner((bool Disabled, bool Pressed, bool Hover) args)
+               {
+                       var separatorImageName = WidgetUtils.GetStatefulImageName(SeparatorImage, args.Disabled, args.Pressed, args.Hover);
+                       return ChromeProvider.GetImage(Separators, separatorImageName) ?? ChromeProvider.GetImage(Separators, SeparatorImage);
+               }
+
                [ObjectCreator.UseCtor]
                public DropDownButtonWidget(ModData modData)
-                       : base(modData) { }
+                       : base(modData)
+               {
+                       getSeparatorImage = new CachedTransform<(bool, bool, bool), Sprite>(GetSeperatorImageInner);
+               }
 
                protected DropDownButtonWidget(DropDownButtonWidget widget)
                        : base(widget)
                {
                        PanelRoot = widget.PanelRoot;
+                       getSeparatorImage = new CachedTransform<(bool, bool, bool), Sprite>(GetSeperatorImageInner);
                }
 
                public override void Draw()
@@ -57,9 +68,7 @@ public override void Draw()
 
                        WidgetUtils.DrawRGBA(arrowImage, stateOffset + new float2(rb.Right - (int)((rb.Height + arrowImage.Size.X) / 2), rb.Top + (int)((rb.Height - arrowImage.Size.Y) / 2)));
 
-                       var separatorImageName = WidgetUtils.GetStatefulImageName(SeparatorImage, isDisabled, Depressed, isHover);
-                       var separatorImage = ChromeProvider.GetImage(Separators, separatorImageName) ?? ChromeProvider.GetImage(Separators, SeparatorImage);
-
+                       var separatorImage = getSeparatorImage.Update((isDisabled, Depressed, isHover));
                        if (separatorImage != null)
                                WidgetUtils.DrawRGBA(separatorImage, stateOffset + new float2(-3, 0) + new float2(rb.Right - rb.Height + 4, rb.Top + (int)((rb.Height - separatorImage.Size.Y) / 2)));
                }

On the other hand, I may be splitting hairs over something that has a completely negligible perf impact, meaning that the extra code uglyness would outweigh any theoretical gains.

[pchote]

15:19 <+pchote> Mesacer_: any thoughts on the perf implications of #18436?
15:58 < Mesacer_> pchote wouldnt your suggestion be good otherwise too depending on what WidgetUtils.GetStatefulImageName and ChromeProvider.GetImage do
16:17 <+pchote> GetStatefulImageName is basically a switch case
16:18 <+pchote> GetImage is two dictionary TryGets in the common case when images exist, four if not
16:30 < Mesacer_> after looking at GetImage i think its worth it to use cachetransform
16:47 <+pchote> ok

[dragunoff]

@pchote Should this optimization be applied to all image queries that use the same pattern?

[pchote]

I'm not sure off the top of my head: probably, but maybe not worthwhile if this means a significant number of changes.

[abcdefg30]

What is the status here?

[dragunoff]

No updates. We could take this PR as-is and take care of the code optimizations in a follow-up. Or close this PR as it doesn't have a lot of value and deal with everything in another PR.

[reaperrr]

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/f4da9ca671541fd89fa7e663e078dcff69bc44d9...ceba7bcf776ba6cc2b39d31ef63b968da692c850