Fixed stack overflow related to panel scrolling and scrollbar auto hiding (and improved build scripts) (#28)

* add a test to recreate the issue

* fixed deploy-demo running on pull requests

* restructure cake script to run tests separately

* actually name the test step correctly

* also run the build task separately

* made all tasks explicitly depend on prepare

* install android workload manually, rather than as part of cake

* Revert "install android workload manually, rather than as part of cake"

This reverts commit 101d6d3eac.

* don't clear a panel's scroll bar max history between area updates

* added changelog entry

.github/workflows/main.yml

@@ -20,10 +20,14 @@ jobs:
         uses: android-actions/setup-android@v3
       - name: Restore tools
         run: dotnet tool restore
-      - name: Run Publish
+      - name: Run Build
+        run: dotnet cake --target Build --ref ${{ github.ref }} --buildNum ${{ github.run_number }}
+      - name: Run Test
         uses: coactions/setup-xvfb@v1
         with:
-          run: dotnet cake --target Publish --ref ${{ github.ref }} --buildNum ${{ github.run_number }}
+          run: dotnet cake --target Test --ref ${{ github.ref }} --buildNum ${{ github.run_number }}
+      - name: Run Publish
+        run: dotnet cake --target Publish --ref ${{ github.ref }} --buildNum ${{ github.run_number }}
         env:
           NUGET_KEY: ${{ secrets.NUGET_KEY }}
           BAGET_KEY: ${{ secrets.BAGET_KEY }}

.github/workflows/web.yml

@@ -23,7 +23,7 @@ jobs:
           include-hidden-files: true
           if-no-files-found: error
   deploy-demo:
-    if: github.event_name == 'push'
+    if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')
     needs: [build-demo]
     runs-on: ubuntu-latest
     steps:

CHANGELOG.md

@@ -30,6 +30,7 @@ Additions
 
 Fixes
 - Fixed tooltips not being bounded correctly for viewports that don't start at the origin
+- Fixed a stack overflow exception when a panel's children have just enough height to cause a scroll bar to appear
 
 ### MLEM.Data
 Improvements

MLEM.Ui/Elements/Panel.cs

@@ -126,13 +126,7 @@ namespace MLEM.Ui.Elements {
                         throw new NotSupportedException($"A panel that handles overflow can't contain non-automatic anchors ({child})");
                 }
             }
-
             base.ForceUpdateArea();
-            if (this.scrollOverflow) {
-                for (var i = 0; i < this.scrollBarMaxHistory.Length; i++)
-                    this.scrollBarMaxHistory[i] = -1;
-            }
-
             this.SetScrollBarStyle();
         }
 

Tests/TestGame.cs

@@ -1,4 +1,5 @@
-using Microsoft.Xna.Framework.Graphics;
+using Microsoft.Xna.Framework;
+using Microsoft.Xna.Framework.Graphics;
 using MLEM.Data.Content;
 using MLEM.Font;
 using MLEM.Startup;
@@ -20,6 +21,10 @@ public class TestGame : MlemGame {
     protected override void LoadContent() {
         base.LoadContent();
         this.RawContent = new RawContentManager(this.Services, this.Content.RootDirectory);
+
+        // make sure that the viewport is always the same size, since RunOneFrame doesn't ensure window size is correct
+        this.UiSystem.Viewport = new Rectangle(0, 0, 1280, 720);
+
         // we use precompiled fonts and kni uses a different asset compilation system, so we just have both stored
         this.UiSystem.Style.Font = new GenericSpriteFont(MlemGame.LoadContent<SpriteFont>(
 #if KNI

Tests/UiTests.cs

@@ -147,6 +147,34 @@ public class UiTests : GameTestFixture {
         }
     }
 
+    [Test]
+    public void TestIssue27([Values(5, 50, 15)] int numChildren) {
+        // Stack overflow related to panel scrolling and scrollbar auto-hiding
+
+        var group = new SquishingGroup(Anchor.TopLeft, Vector2.One);
+
+        var centerGroup = new ScissorGroup(Anchor.TopCenter, Vector2.One);
+        var centerPanel = new Panel(Anchor.TopRight, Vector2.One);
+        centerPanel.DrawColor = Color.Red;
+        centerPanel.Padding = new MLEM.Maths.Padding(5);
+        centerGroup.AddChild(centerPanel);
+        group.AddChild(centerGroup);
+
+        var leftColumn = new Panel(Anchor.TopLeft, new Vector2(500, 1), scrollOverflow: true);
+        group.AddChild(leftColumn);
+        for (var i = 0; i < numChildren; i++) {
+            var c = new Panel(Anchor.AutoLeft, new Vector2(1, 30));
+            c.DrawColor = Color.Green;
+            c.Padding = new MLEM.Maths.Padding(5);
+            leftColumn.AddChild(c);
+        }
+
+        var bottomPane = new Panel(Anchor.BottomCenter, new Vector2(1, 500));
+        group.AddChild(bottomPane);
+
+        this.AddAndUpdate(group, out _, out _);
+    }
+
     private void AddAndUpdate(Element element, out TimeSpan addTime, out TimeSpan updateTime) {
         foreach (var root in this.Game.UiSystem.GetRootElements())
             this.Game.UiSystem.Remove(root.Name);

build.cake

@@ -36,7 +36,7 @@ Task("Build").IsDependentOn("Prepare").Does(() =>{
     DotNetBuild("MLEM.KNI.sln", settings);
 });
 
-Task("Test").IsDependentOn("Build").Does(() => {
+Task("Test").IsDependentOn("Prepare").Does(() => {
     var settings = new DotNetTestSettings {
         Configuration = config,
         Collectors = {"XPlat Code Coverage"},
@@ -47,7 +47,7 @@ Task("Test").IsDependentOn("Build").Does(() => {
     DotNetTest("MLEM.KNI.sln", settings);
 });
 
-Task("Pack").IsDependentOn("Test").Does(() => {
+Task("Pack").IsDependentOn("Prepare").Does(() => {
     var settings = new DotNetPackSettings {
         Configuration = config,
         ArgumentCustomization = args => args.Append($"/p:Version={version}")
@@ -76,21 +76,21 @@ Task("Push").WithCriteria(gitRef == "refs/heads/main" || gitRef.StartsWith("refs
     DotNetNuGetPush("**/MLEM*.nupkg", settings);
 });
 
-Task("Document").Does(() => {
+Task("Document").IsDependentOn("Prepare").Does(() => {
     DocFxMetadata("Docs/docfx.json");
     DocFxBuild("Docs/docfx.json");
     if (serve)
         DocFxServe("Docs/_site");
 });
 
-Task("PublishWeb").Does(() => {
+Task("PublishWeb").IsDependentOn("Prepare").Does(() => {
     DotNetPublish("Demos.Web/Demos.Web.KNI.csproj", new DotNetPublishSettings {
         Configuration = config,
         ArgumentCustomization = args => args.Append($"/p:Version={version}")
     });
 });
 
-Task("Default").IsDependentOn("Pack");
+Task("Default").IsDependentOn("Build").IsDependentOn("Test").IsDependentOn("Pack");
 Task("Publish").IsDependentOn("Push");
 
 RunTarget(target);