From 141f9b02cbf600254000ad82a4e8a20275aa5e0a Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 4 Nov 2024 00:20:05 -0500 Subject: [PATCH 01/26] Disable `OnScreenKeyboardOverlapsGameWindow` on iOS when hardware keyboard is attached --- osu.Framework.iOS/IOSGameHost.cs | 3 ++- osu.Framework/Platform/GameHost.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Framework.iOS/IOSGameHost.cs b/osu.Framework.iOS/IOSGameHost.cs index 813d6133e6..91c3401a0a 100644 --- a/osu.Framework.iOS/IOSGameHost.cs +++ b/osu.Framework.iOS/IOSGameHost.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using Foundation; +using GameController; using osu.Framework.Configuration; using osu.Framework.Extensions; using osu.Framework.Extensions.ObjectExtensions; @@ -38,7 +39,7 @@ protected override void SetupConfig(IDictionary defaul base.SetupConfig(defaultOverrides); } - public override bool OnScreenKeyboardOverlapsGameWindow => true; + public override bool OnScreenKeyboardOverlapsGameWindow => !OperatingSystem.IsIOSVersionAtLeast(14) || GCKeyboard.CoalescedKeyboard == null; public override bool CanExit => false; diff --git a/osu.Framework/Platform/GameHost.cs b/osu.Framework/Platform/GameHost.cs index e5ed5aa6b5..4eab59a416 100644 --- a/osu.Framework/Platform/GameHost.cs +++ b/osu.Framework/Platform/GameHost.cs @@ -122,7 +122,8 @@ public abstract class GameHost : IIpcHost, IDisposable public event Func MessageReceived; /// - /// Whether the on screen keyboard covers a portion of the game window when presented to the user. + /// Whether the on-screen keyboard covers a portion of the game window when presented to the user. + /// This is usually true on mobile platforms, but may change to false if a hardware keyboard is connected. /// public virtual bool OnScreenKeyboardOverlapsGameWindow => false; From 0b4f7c15c4d0bdf30c7574e75b95d39ca88a9b11 Mon Sep 17 00:00:00 2001 From: Jumprocks Date: Mon, 4 Nov 2024 22:31:03 -0700 Subject: [PATCH 02/26] Add failing test case --- osu.Framework.Tests/Graphics/RendererTest.cs | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/osu.Framework.Tests/Graphics/RendererTest.cs b/osu.Framework.Tests/Graphics/RendererTest.cs index 1f77e8e1cb..fd84554e2c 100644 --- a/osu.Framework.Tests/Graphics/RendererTest.cs +++ b/osu.Framework.Tests/Graphics/RendererTest.cs @@ -26,5 +26,28 @@ public void TestWhitePixelReuseUpdatesTextureWrapping() Assert.That(renderer.CurrentWrapModeS, Is.EqualTo(WrapMode.None)); Assert.That(renderer.CurrentWrapModeS, Is.EqualTo(WrapMode.None)); } + + [Test] + public void TestTextureAtlasReuseUpdatesTextureWrapping() + { + DummyRenderer renderer = new DummyRenderer(); + + TextureAtlas atlas = new TextureAtlas(renderer, 1024, 1024); + + Texture textureWrapNone = atlas.Add(100, 100, WrapMode.None, WrapMode.None)!; + Texture textureWrapClamp = atlas.Add(100, 100, WrapMode.ClampToEdge, WrapMode.ClampToEdge)!; + + renderer.BindTexture(textureWrapNone, 0, null, null); + Assert.That(renderer.CurrentWrapModeS, Is.EqualTo(WrapMode.None)); + Assert.That(renderer.CurrentWrapModeT, Is.EqualTo(WrapMode.None)); + + renderer.BindTexture(textureWrapClamp, 0, null, null); + Assert.That(renderer.CurrentWrapModeS, Is.EqualTo(WrapMode.ClampToEdge)); + Assert.That(renderer.CurrentWrapModeT, Is.EqualTo(WrapMode.ClampToEdge)); + + renderer.BindTexture(textureWrapNone, 0, null, null); + Assert.That(renderer.CurrentWrapModeS, Is.EqualTo(WrapMode.None)); + Assert.That(renderer.CurrentWrapModeT, Is.EqualTo(WrapMode.None)); + } } } From 745989499886969ce9a8bd08215d62f4503bfce6 Mon Sep 17 00:00:00 2001 From: Jumprocks Date: Mon, 4 Nov 2024 22:32:22 -0700 Subject: [PATCH 03/26] Fix texture wrapping not updating when atlas already bound --- osu.Framework/Graphics/Rendering/Renderer.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Framework/Graphics/Rendering/Renderer.cs b/osu.Framework/Graphics/Rendering/Renderer.cs index 8066d379ed..01a8a1f39a 100644 --- a/osu.Framework/Graphics/Rendering/Renderer.cs +++ b/osu.Framework/Graphics/Rendering/Renderer.cs @@ -818,7 +818,10 @@ public bool BindTexture(Texture texture, int unit, WrapMode? wrapModeS, WrapMode public bool BindTexture(INativeTexture texture, int unit = 0, WrapMode wrapModeS = WrapMode.None, WrapMode wrapModeT = WrapMode.None) { if (lastActiveTextureUnit == unit && lastBoundTexture[unit] == texture) + { + setWrapMode(wrapModeS, wrapModeT); return true; + } FlushCurrentBatch(FlushBatchSource.BindTexture); From 0303aa6a0865a564edc89cb1a6b2fa9a8af64f49 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 5 Nov 2024 15:57:02 +0900 Subject: [PATCH 04/26] Add attribute to hide arbitrary drawables from draw visualiser --- .../Graphics/Visualisation/DrawVisualiser.cs | 32 +++++++++++-------- .../DrawVisualiserHiddenAttribute.cs | 13 ++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 osu.Framework/Graphics/Visualisation/DrawVisualiserHiddenAttribute.cs diff --git a/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs b/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs index c66615f740..2362c7c73e 100644 --- a/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs +++ b/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs @@ -207,6 +207,8 @@ protected override void Update() overlay.Target = Searching ? cursorTarget : inputManager.HoveredDrawables.OfType().FirstOrDefault()?.Target; } + private static readonly Dictionary is_type_valid_target_cache = new Dictionary(); + private void updateCursorTarget() { Drawable drawableTarget = null; @@ -268,30 +270,32 @@ void findTarget(Drawable drawable) if (!validForTarget(drawable)) return; - // Special case for full-screen overlays that act as input receptors, but don't display anything - if (!hasCustomDrawNode(drawable)) - return; - drawableTarget = drawable; } } // Valid if the drawable contains the mouse position and the position wouldn't be masked by the parent bool validForTarget(Drawable drawable) - => drawable.ScreenSpaceDrawQuad.Contains(inputManager.CurrentState.Mouse.Position) - && maskingQuad?.Contains(inputManager.CurrentState.Mouse.Position) != false; - } + { + if (!drawable.ScreenSpaceDrawQuad.Contains(inputManager.CurrentState.Mouse.Position) + || maskingQuad?.Contains(inputManager.CurrentState.Mouse.Position) == false) + { + return false; + } - private static readonly Dictionary has_custom_drawnode_cache = new Dictionary(); + Type type = drawable.GetType(); - private bool hasCustomDrawNode(Drawable drawable) - { - var type = drawable.GetType(); + if (is_type_valid_target_cache.TryGetValue(type, out bool valid)) + return valid; - if (has_custom_drawnode_cache.TryGetValue(type, out bool existing)) - return existing; + // Exclude "overlay" objects (Component/etc) that don't draw anything and don't override CreateDrawNode(). + valid = type.GetMethod(nameof(CreateDrawNode), BindingFlags.Instance | BindingFlags.NonPublic)?.DeclaringType != typeof(Drawable); - return has_custom_drawnode_cache[type] = type.GetMethod(nameof(CreateDrawNode), BindingFlags.Instance | BindingFlags.NonPublic)?.DeclaringType != typeof(Drawable); + // Exclude objects that specify they should be hidden anyway. + valid &= !type.GetCustomAttributes(true).Any(); + + return is_type_valid_target_cache[type] = valid; + } } public bool Searching { get; private set; } diff --git a/osu.Framework/Graphics/Visualisation/DrawVisualiserHiddenAttribute.cs b/osu.Framework/Graphics/Visualisation/DrawVisualiserHiddenAttribute.cs new file mode 100644 index 0000000000..c5c62d683c --- /dev/null +++ b/osu.Framework/Graphics/Visualisation/DrawVisualiserHiddenAttribute.cs @@ -0,0 +1,13 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; + +namespace osu.Framework.Graphics.Visualisation +{ + /// + /// Indicates that instances of this type or any subtype should not be valid targets for the draw visualiser. + /// + [AttributeUsage(AttributeTargets.Class)] + public class DrawVisualiserHiddenAttribute : Attribute; +} From b8e9e0b96d03410eb055d8bb27d1a123fadc20e6 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 6 Nov 2024 14:02:56 +0900 Subject: [PATCH 05/26] Update GHA --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6583d36179..5905850e62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,7 +96,7 @@ jobs: # Attempt to upload results even if test fails. # https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#always - name: Upload Test Results - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: ${{ always() }} with: name: osu-framework-test-results-${{matrix.os.prettyname}}-${{matrix.threadingMode}}-${{matrix.os.configuration}} From c7d1e720cf83b10d686f9a1e6ad4dfa7f8a1a2df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 6 Nov 2024 11:08:05 +0100 Subject: [PATCH 06/26] Add better support for adding form file parameters to web requests Something I ended up needing in the course of BSS work. The previous implementation would treat files as a straight dictionary, as in you could have only one file with a given parameter name in a request. This isn't how `multipart/form-data` really works; you're allowed to send multiple files in scope of a single parameter. Additionally, the request did not permit specifying a filename, which is yet another thing I need for BSS purposes. I don't believe this method has any real consumers, yet anyhow: Breaking changes ---------------- `WebRequest.AddFile()` has been altered to provide an API closer to what the HTTP specs intended, and to allow sending multiple files for a given request parameter. To that end: - The signature of the method has changed from ```csharp public void AddFile(string name, byte[] data); ``` to ```csharp public void AddFile(string paramName, byte[] data, string filename = "blob"); ``` - Calling `.AddFile()` with the same first parameter (formerly called `name`, now called `paramName`) twice will result in two files being sent in the request, rather than the second call overwriting the file added by the first one. - If the `filename` parameter is not explicitly specified in the method call, the filename given in the `Content-Disposition` header of the multipart request has changed from being equal to the parameter name to `blob`. This is consistent with JavaScript `FormData` API behaviour. --- osu.Framework/IO/Network/WebRequest.cs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/osu.Framework/IO/Network/WebRequest.cs b/osu.Framework/IO/Network/WebRequest.cs index 4fff194c84..e3d5a91b48 100644 --- a/osu.Framework/IO/Network/WebRequest.cs +++ b/osu.Framework/IO/Network/WebRequest.cs @@ -119,7 +119,7 @@ public class WebRequest : IDisposable /// /// FILE parameters. /// - private readonly IDictionary files = new Dictionary(); + private readonly List files = new List(); /// /// The request headers. @@ -349,9 +349,9 @@ private async Task internalPerform(CancellationToken cancellationToken = default foreach (var p in files) { - var byteContent = new ByteArrayContent(p.Value); + var byteContent = new ByteArrayContent(p.Content); byteContent.Headers.Add("Content-Type", "application/octet-stream"); - formData.Add(byteContent, p.Key, p.Key); + formData.Add(byteContent, p.ParamName, p.Filename); } postContent = await formData.ReadAsStreamAsync(linkedToken.Token).ConfigureAwait(false); @@ -662,17 +662,21 @@ public void AddRaw(Stream stream) } /// - /// Add a new FILE parameter to this request. Replaces any existing file with the same name. + /// Add a new FILE parameter to this request. /// This may not be used in conjunction with . GET requests may not contain files. /// - /// The name of the file. This becomes the name of the file in a multi-part form POST content. + /// The name of the form parameter of the request that the file relates to. /// The file data. - public void AddFile(string name, byte[] data) + /// + /// The filename of the file to be sent to be reported to the server in the Content-Disposition header. + /// blob is used by default if omitted, to mirror browser behaviour. + /// + public void AddFile(string paramName, byte[] data, string filename = "blob") { - ArgumentNullException.ThrowIfNull(name); + ArgumentNullException.ThrowIfNull(paramName); ArgumentNullException.ThrowIfNull(data); - files[name] = data; + files.Add(new FormFile(paramName, data, filename)); } /// @@ -931,5 +935,7 @@ protected override void Dispose(bool disposing) baseStream.Dispose(); } } + + private record struct FormFile(string ParamName, byte[] Content, string Filename); } } From 1a09aafb185617467f6bf94467d5785b981608ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 8 Nov 2024 11:27:12 +0100 Subject: [PATCH 07/26] Fix SDL3 attempting to render IME composition text Is not necessary since we handle this ourselves. Additionally the Windows implementation of this looks abhorrent. --- osu.Framework/Platform/SDL3/SDL3Window.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Framework/Platform/SDL3/SDL3Window.cs b/osu.Framework/Platform/SDL3/SDL3Window.cs index ad30b20ad9..56a006f2df 100644 --- a/osu.Framework/Platform/SDL3/SDL3Window.cs +++ b/osu.Framework/Platform/SDL3/SDL3Window.cs @@ -209,6 +209,7 @@ public virtual void Create() SDL_SetHint(SDL_HINT_MOUSE_RELATIVE_MODE_CENTER, "0"u8); SDL_SetHint(SDL_HINT_TOUCH_MOUSE_EVENTS, "0"u8); // disable touch events generating synthetic mouse events on desktop platforms SDL_SetHint(SDL_HINT_MOUSE_TOUCH_EVENTS, "0"u8); // disable mouse events generating synthetic touch events on mobile platforms + SDL_SetHint(SDL_HINT_IME_IMPLEMENTED_UI, "composition"u8); SDLWindowHandle = SDL_CreateWindow(title, Size.Width, Size.Height, flags); From f3bdd4e916e060405d1ad846c806011fc191ac49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 8 Nov 2024 12:54:26 +0100 Subject: [PATCH 08/26] Remove duplicated SDL hint spec Overlooked by yours truly. Pushing to master to reduce ceremony. --- osu.Framework/Platform/SDL3/SDL3Window.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Framework/Platform/SDL3/SDL3Window.cs b/osu.Framework/Platform/SDL3/SDL3Window.cs index 56a006f2df..5944581b10 100644 --- a/osu.Framework/Platform/SDL3/SDL3Window.cs +++ b/osu.Framework/Platform/SDL3/SDL3Window.cs @@ -205,7 +205,6 @@ public virtual void Create() flags |= graphicsSurface.Type.ToFlags(); SDL_SetHint(SDL_HINT_WINDOWS_CLOSE_ON_ALT_F4, "0"u8); - SDL_SetHint(SDL_HINT_IME_IMPLEMENTED_UI, "0"u8); SDL_SetHint(SDL_HINT_MOUSE_RELATIVE_MODE_CENTER, "0"u8); SDL_SetHint(SDL_HINT_TOUCH_MOUSE_EVENTS, "0"u8); // disable touch events generating synthetic mouse events on desktop platforms SDL_SetHint(SDL_HINT_MOUSE_TOUCH_EVENTS, "0"u8); // disable mouse events generating synthetic touch events on mobile platforms From 456b586a66bfe4b807483ef729baa11941e0b060 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Sat, 9 Nov 2024 05:05:57 -0500 Subject: [PATCH 09/26] Move on-screen keyboard check to `SDLGameHost` --- osu.Framework.iOS/IOSGameHost.cs | 3 --- osu.Framework.iOS/IOSWindow.cs | 1 - osu.Framework/Platform/ISDLWindow.cs | 1 + osu.Framework/Platform/SDL2/SDL2Window.cs | 2 ++ osu.Framework/Platform/SDL3/SDL3Window.cs | 2 ++ osu.Framework/Platform/SDLGameHost.cs | 2 ++ 6 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Framework.iOS/IOSGameHost.cs b/osu.Framework.iOS/IOSGameHost.cs index 91c3401a0a..7b696fd97f 100644 --- a/osu.Framework.iOS/IOSGameHost.cs +++ b/osu.Framework.iOS/IOSGameHost.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.IO; using Foundation; -using GameController; using osu.Framework.Configuration; using osu.Framework.Extensions; using osu.Framework.Extensions.ObjectExtensions; @@ -39,8 +38,6 @@ protected override void SetupConfig(IDictionary defaul base.SetupConfig(defaultOverrides); } - public override bool OnScreenKeyboardOverlapsGameWindow => !OperatingSystem.IsIOSVersionAtLeast(14) || GCKeyboard.CoalescedKeyboard == null; - public override bool CanExit => false; public override Storage GetStorage(string path) => new IOSStorage(path, this); diff --git a/osu.Framework.iOS/IOSWindow.cs b/osu.Framework.iOS/IOSWindow.cs index 5444664a88..1583ffccb8 100644 --- a/osu.Framework.iOS/IOSWindow.cs +++ b/osu.Framework.iOS/IOSWindow.cs @@ -11,7 +11,6 @@ using osu.Framework.Graphics; using osu.Framework.Platform; using osu.Framework.Platform.SDL3; -using SDL; using static SDL.SDL3; using UIKit; diff --git a/osu.Framework/Platform/ISDLWindow.cs b/osu.Framework/Platform/ISDLWindow.cs index 66f627e28e..640cd8b099 100644 --- a/osu.Framework/Platform/ISDLWindow.cs +++ b/osu.Framework/Platform/ISDLWindow.cs @@ -39,6 +39,7 @@ internal interface ISDLWindow : IWindow bool MouseAutoCapture { set; } bool RelativeMouseMode { get; set; } bool CapsLockPressed { get; } + bool KeyboardAttached { get; } void UpdateMousePosition(Vector2 position); diff --git a/osu.Framework/Platform/SDL2/SDL2Window.cs b/osu.Framework/Platform/SDL2/SDL2Window.cs index 7a62f84d7c..1efa878477 100644 --- a/osu.Framework/Platform/SDL2/SDL2Window.cs +++ b/osu.Framework/Platform/SDL2/SDL2Window.cs @@ -165,6 +165,8 @@ internal SDL_SysWMinfo GetWindowSystemInformation() public bool CapsLockPressed => SDL_GetModState().HasFlagFast(SDL_Keymod.KMOD_CAPS); + public bool KeyboardAttached => true; // SDL2 has no way of knowing whether a keyboard is attached, assume true. + // references must be kept to avoid GC, see https://stackoverflow.com/a/6193914 [UsedImplicitly] diff --git a/osu.Framework/Platform/SDL3/SDL3Window.cs b/osu.Framework/Platform/SDL3/SDL3Window.cs index ad30b20ad9..39457dee9d 100644 --- a/osu.Framework/Platform/SDL3/SDL3Window.cs +++ b/osu.Framework/Platform/SDL3/SDL3Window.cs @@ -145,6 +145,8 @@ public IntPtr DisplayHandle public bool CapsLockPressed => SDL_GetModState().HasFlagFast(SDL_Keymod.SDL_KMOD_CAPS); + public bool KeyboardAttached => SDL_HasKeyboard(); + /// /// Represents a handle to this instance, used for unmanaged callbacks. /// diff --git a/osu.Framework/Platform/SDLGameHost.cs b/osu.Framework/Platform/SDLGameHost.cs index 41e237db9b..042a7fc9a9 100644 --- a/osu.Framework/Platform/SDLGameHost.cs +++ b/osu.Framework/Platform/SDLGameHost.cs @@ -20,6 +20,8 @@ public abstract class SDLGameHost : GameHost { public override bool CapsLockEnabled => (Window as ISDLWindow)?.CapsLockPressed == true; + public override bool OnScreenKeyboardOverlapsGameWindow => (Window as ISDLWindow)?.KeyboardAttached == false; + protected SDLGameHost(string gameName, HostOptions? options = null) : base(gameName, options) { From 23913b726a26839f5f33871743e18762d35b6e7b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 Nov 2024 17:41:10 +0900 Subject: [PATCH 10/26] Allow customising `ScrollContainer`'s scrollbar mapping For use in osu!. --- osu.Framework/Graphics/Containers/ScrollContainer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Framework/Graphics/Containers/ScrollContainer.cs b/osu.Framework/Graphics/Containers/ScrollContainer.cs index eae4c968ee..e838aa683f 100644 --- a/osu.Framework/Graphics/Containers/ScrollContainer.cs +++ b/osu.Framework/Graphics/Containers/ScrollContainer.cs @@ -409,7 +409,7 @@ protected override bool OnScroll(ScrollEvent e) return true; } - private void onScrollbarMovement(float value) => OnUserScroll(Clamp(fromScrollbarPosition(value)), false); + private void onScrollbarMovement(float value) => OnUserScroll(Clamp(FromScrollbarPosition(value)), false); /// /// Immediately offsets the current and target scroll position. @@ -576,12 +576,12 @@ protected override void UpdateAfterChildren() if (ScrollDirection == Direction.Horizontal) { - Scrollbar.X = toScrollbarPosition(Current); + Scrollbar.X = ToScrollbarPosition(Current); ScrollContent.X = -Current + ScrollableExtent * ScrollContent.RelativeAnchorPosition.X; } else { - Scrollbar.Y = toScrollbarPosition(Current); + Scrollbar.Y = ToScrollbarPosition(Current); ScrollContent.Y = -Current + ScrollableExtent * ScrollContent.RelativeAnchorPosition.Y; } } @@ -591,7 +591,7 @@ protected override void UpdateAfterChildren() /// /// The absolute scroll position (e.g. ). /// The scrollbar position. - private float toScrollbarPosition(float scrollPosition) + protected virtual float ToScrollbarPosition(float scrollPosition) { if (Precision.AlmostEquals(0, ScrollableExtent)) return 0; @@ -604,7 +604,7 @@ private float toScrollbarPosition(float scrollPosition) /// /// The scrollbar position. /// The absolute scroll position. - private float fromScrollbarPosition(float scrollbarPosition) + protected virtual float FromScrollbarPosition(float scrollbarPosition) { if (Precision.AlmostEquals(0, ScrollbarMovementExtent)) return 0; From 3bab44a584201ace24508a6d6a2177412110ba7f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 Nov 2024 19:08:52 +0900 Subject: [PATCH 11/26] Make `ScrollbarMovementExtent` `protected` --- osu.Framework/Graphics/Containers/ScrollContainer.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Framework/Graphics/Containers/ScrollContainer.cs b/osu.Framework/Graphics/Containers/ScrollContainer.cs index e838aa683f..dbd89fed1c 100644 --- a/osu.Framework/Graphics/Containers/ScrollContainer.cs +++ b/osu.Framework/Graphics/Containers/ScrollContainer.cs @@ -136,7 +136,10 @@ public bool ScrollbarOverlapsContent /// /// The maximum distance that the scrollbar can move in the scroll direction. /// - public float ScrollbarMovementExtent => Math.Max(DisplayableContent - Scrollbar.DrawSize[ScrollDim], 0); + /// + /// May not be accurate to actual display of scrollbar if or are overridden. + /// + protected float ScrollbarMovementExtent => Math.Max(DisplayableContent - Scrollbar.DrawSize[ScrollDim], 0); /// /// Clamp a value to the available scroll range. From b27ed6474e57c52b1d97d28c3b624406db66c4db Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 11 Nov 2024 20:09:30 +0900 Subject: [PATCH 12/26] Allow CachedModelDependencyContainer to cache models with non-bindable fields --- .../Reflection/CachedModelDependenciesTest.cs | 60 ++-------------- .../CachedModelDependenciesTest.cs | 60 ++-------------- .../CachedModelDependencyContainer.cs | 69 ++++++------------- 3 files changed, 34 insertions(+), 155 deletions(-) diff --git a/osu.Framework.Tests/Dependencies/Reflection/CachedModelDependenciesTest.cs b/osu.Framework.Tests/Dependencies/Reflection/CachedModelDependenciesTest.cs index 8c27434c91..02297e5bde 100644 --- a/osu.Framework.Tests/Dependencies/Reflection/CachedModelDependenciesTest.cs +++ b/osu.Framework.Tests/Dependencies/Reflection/CachedModelDependenciesTest.cs @@ -1,9 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - -using System; using System.Diagnostics.CodeAnalysis; using NUnit.Framework; using osu.Framework.Allocation; @@ -15,24 +12,6 @@ namespace osu.Framework.Tests.Dependencies.Reflection [SuppressMessage("Performance", "OFSG001:Class contributes to dependency injection and should be partial")] public class CachedModelDependenciesTest { - [Test] - public void TestModelWithNonBindableFieldsFails() - { - IReadOnlyDependencyContainer unused; - - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - } - - [Test] - public void TestModelWithNonReadOnlyFieldsFails() - { - IReadOnlyDependencyContainer unused; - - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - } - [Test] public void TestSettingNoModelResolvesDefault() { @@ -195,7 +174,7 @@ public void TestSetModelToNullAfterResolved() var model = new FieldModel { Bindable = { Value = 2 } }; - var dependencies = new CachedModelDependencyContainer(null) + var dependencies = new CachedModelDependencyContainer(null) { Model = { Value = model } }; @@ -248,7 +227,7 @@ public void TestResolveIndividualProperties() BindableString = { Value = "3" } }; - var dependencies = new CachedModelDependencyContainer(null) + var dependencies = new CachedModelDependencyContainer(null) { Model = { Value = model1 } }; @@ -269,33 +248,6 @@ public void TestResolveIndividualProperties() Assert.AreEqual(null, resolver.BindableString.Value); } - private class NonBindablePublicFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 649 - public readonly int FailingField; -#pragma warning restore 649 - } - - private class NonBindablePrivateFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 169 - private readonly int failingField; -#pragma warning restore 169 - } - - private class NonReadOnlyFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 649 - public Bindable Bindable; -#pragma warning restore 649 - } - - private class PropertyModel : IDependencyInjectionCandidate - { - // ReSharper disable once UnusedMember.Local - public Bindable Bindable { get; private set; } - } - private class FieldModel : IDependencyInjectionCandidate { [Cached] @@ -311,22 +263,22 @@ private class DerivedFieldModel : FieldModel private class FieldModelResolver : IDependencyInjectionCandidate { [Resolved] - public FieldModel Model { get; private set; } + public FieldModel Model { get; private set; } = null!; } private class DerivedFieldModelResolver : IDependencyInjectionCandidate { [Resolved] - public DerivedFieldModel Model { get; private set; } + public DerivedFieldModel Model { get; private set; } = null!; } private class DerivedFieldModelPropertyResolver : IDependencyInjectionCandidate { [Resolved(typeof(DerivedFieldModel))] - public Bindable Bindable { get; private set; } + public Bindable Bindable { get; private set; } = null!; [Resolved(typeof(DerivedFieldModel))] - public Bindable BindableString { get; private set; } + public Bindable BindableString { get; private set; } = null!; } } } diff --git a/osu.Framework.Tests/Dependencies/SourceGeneration/CachedModelDependenciesTest.cs b/osu.Framework.Tests/Dependencies/SourceGeneration/CachedModelDependenciesTest.cs index 8555380a72..84529a4ae5 100644 --- a/osu.Framework.Tests/Dependencies/SourceGeneration/CachedModelDependenciesTest.cs +++ b/osu.Framework.Tests/Dependencies/SourceGeneration/CachedModelDependenciesTest.cs @@ -1,9 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - -using System; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -13,24 +10,6 @@ namespace osu.Framework.Tests.Dependencies.SourceGeneration [TestFixture] public partial class CachedModelDependenciesTest { - [Test] - public void TestModelWithNonBindableFieldsFails() - { - IReadOnlyDependencyContainer unused; - - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - } - - [Test] - public void TestModelWithNonReadOnlyFieldsFails() - { - IReadOnlyDependencyContainer unused; - - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - Assert.Throws(() => unused = new CachedModelDependencyContainer(null)); - } - [Test] public void TestSettingNoModelResolvesDefault() { @@ -193,7 +172,7 @@ public void TestSetModelToNullAfterResolved() var model = new FieldModel { Bindable = { Value = 2 } }; - var dependencies = new CachedModelDependencyContainer(null) + var dependencies = new CachedModelDependencyContainer(null) { Model = { Value = model } }; @@ -246,7 +225,7 @@ public void TestResolveIndividualProperties() BindableString = { Value = "3" } }; - var dependencies = new CachedModelDependencyContainer(null) + var dependencies = new CachedModelDependencyContainer(null) { Model = { Value = model1 } }; @@ -267,33 +246,6 @@ public void TestResolveIndividualProperties() Assert.AreEqual(null, resolver.BindableString.Value); } - private partial class NonBindablePublicFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 649 - public readonly int FailingField; -#pragma warning restore 649 - } - - private partial class NonBindablePrivateFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 169 - private readonly int failingField; -#pragma warning restore 169 - } - - private partial class NonReadOnlyFieldModel : IDependencyInjectionCandidate - { -#pragma warning disable 649 - public Bindable Bindable; -#pragma warning restore 649 - } - - private partial class PropertyModel : IDependencyInjectionCandidate - { - // ReSharper disable once UnusedMember.Local - public Bindable Bindable { get; private set; } - } - private partial class FieldModel : IDependencyInjectionCandidate { [Cached] @@ -309,22 +261,22 @@ private partial class DerivedFieldModel : FieldModel private partial class FieldModelResolver : IDependencyInjectionCandidate { [Resolved] - public FieldModel Model { get; private set; } + public FieldModel Model { get; private set; } = null!; } private partial class DerivedFieldModelResolver : IDependencyInjectionCandidate { [Resolved] - public DerivedFieldModel Model { get; private set; } + public DerivedFieldModel Model { get; private set; } = null!; } private partial class DerivedFieldModelPropertyResolver : IDependencyInjectionCandidate { [Resolved(typeof(DerivedFieldModel))] - public Bindable Bindable { get; private set; } + public Bindable Bindable { get; private set; } = null!; [Resolved(typeof(DerivedFieldModel))] - public Bindable BindableString { get; private set; } + public Bindable BindableString { get; private set; } = null!; } } } diff --git a/osu.Framework/Allocation/CachedModelDependencyContainer.cs b/osu.Framework/Allocation/CachedModelDependencyContainer.cs index ec3f0768f1..12f37dbe87 100644 --- a/osu.Framework/Allocation/CachedModelDependencyContainer.cs +++ b/osu.Framework/Allocation/CachedModelDependencyContainer.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Reflection; using osu.Framework.Bindables; @@ -19,7 +17,7 @@ namespace osu.Framework.Allocation /// /// The type of the model to cache. Must contain only fields or auto-properties. public class CachedModelDependencyContainer : IReadOnlyDependencyContainer - where TModel : class, IDependencyInjectionCandidate, new() + where TModel : class?, IDependencyInjectionCandidate?, new() { private const BindingFlags activator_flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly; @@ -32,17 +30,16 @@ public class CachedModelDependencyContainer : IReadOnlyDependencyContain public readonly Bindable Model = new Bindable(); private readonly TModel shadowModel = new TModel(); - - private readonly IReadOnlyDependencyContainer parent; + private readonly IReadOnlyDependencyContainer? parent; private readonly IReadOnlyDependencyContainer shadowDependencies; - public CachedModelDependencyContainer(IReadOnlyDependencyContainer parent) + public CachedModelDependencyContainer(IReadOnlyDependencyContainer? parent) { this.parent = parent; shadowDependencies = DependencyActivator.MergeDependencies(shadowModel, null, new CacheInfo(parent: typeof(TModel))); - TModel currentModel = null; + TModel? currentModel = null; Model.BindValueChanged(e => { // When setting a null model, we actually want to reset the shadow model to a default state @@ -55,9 +52,9 @@ public CachedModelDependencyContainer(IReadOnlyDependencyContainer parent) }); } - public object Get(Type type) => Get(type, default); + public object? Get(Type type) => Get(type, default); - public object Get(Type type, CacheInfo info) + public object? Get(Type type, CacheInfo info) { if (info.Parent == null) return type == typeof(TModel) ? createChildShadowModel() : parent?.Get(type, info); @@ -87,65 +84,43 @@ private TModel createChildShadowModel() /// The shadow model to update. /// The model to unbind from. /// The model to bind to. - private void updateShadowModel(TModel targetShadowModel, TModel lastModel, TModel newModel) + private void updateShadowModel(TModel targetShadowModel, TModel? lastModel, TModel newModel) { - // Due to static-constructor checks, we are guaranteed that all fields will be IBindable - - foreach (var type in typeof(TModel).EnumerateBaseTypes()) + if (lastModel != null) { - foreach (var field in type.GetFields(activator_flags)) + foreach (var type in typeof(TModel).EnumerateBaseTypes()) { - perform(targetShadowModel, field, lastModel, (shadowProp, modelProp) => shadowProp.UnbindFrom(modelProp)); + foreach (var field in type.GetFields(activator_flags)) + perform(field, targetShadowModel, lastModel, (shadowProp, modelProp) => shadowProp.UnbindFrom(modelProp)); } } foreach (var type in typeof(TModel).EnumerateBaseTypes()) { foreach (var field in type.GetFields(activator_flags)) - { - perform(targetShadowModel, field, newModel, (shadowProp, modelProp) => shadowProp.BindTo(modelProp)); - } + perform(field, targetShadowModel, newModel, (shadowProp, modelProp) => shadowProp.BindTo(modelProp)); } } /// /// Perform an arbitrary action across a shadow model and model. /// - private void perform(TModel targetShadowModel, MemberInfo member, TModel target, Action action) + private static void perform(FieldInfo field, TModel shadowModel, TModel targetModel, Action action) { - if (target == null) return; + IBindable? shadowBindable = null; + IBindable? targetBindable = null; - switch (member) + try { - case PropertyInfo pi: - action((IBindable)pi.GetValue(targetShadowModel), (IBindable)pi.GetValue(target)); - break; - - case FieldInfo fi: - action((IBindable)fi.GetValue(targetShadowModel), (IBindable)fi.GetValue(target)); - break; + shadowBindable = field.GetValue(shadowModel) as IBindable; + targetBindable = field.GetValue(targetModel) as IBindable; } - } - - static CachedModelDependencyContainer() - { - foreach (var type in typeof(TModel).EnumerateBaseTypes()) + catch { - foreach (var field in type.GetFields(activator_flags)) - { - if (!typeof(IBindable).IsAssignableFrom(field.FieldType)) - { - throw new InvalidOperationException($"\"{field.DeclaringType}.{field.Name}\" does not subclass {nameof(IBindable)}. " - + $"All fields of {typeof(TModel)} must subclass {nameof(IBindable)} to be used in a {nameof(CachedModelDependencyContainer)}."); - } - - if (!field.IsInitOnly) - { - throw new InvalidOperationException($"\"{field.DeclaringType}.{field.Name}\" is not readonly. " - + $"All fields of {typeof(TModel)} must be readonly to be used in a {nameof(CachedModelDependencyContainer)}."); - } - } } + + if (shadowBindable != null && targetBindable != null) + action(shadowBindable, targetBindable); } } } From 94bd9c2f73f0bb72af93ef175e4c169d1440c03b Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 11 Nov 2024 22:56:46 +0900 Subject: [PATCH 13/26] Refactor step buttons + enable NRT --- .../Visual/Testing/TestSceneStepButton.cs | 45 ++++- .../Visual/Testing/TestSceneTest.cs | 5 +- .../Testing/Drawables/Steps/AssertButton.cs | 19 +-- .../Testing/Drawables/Steps/LabelStep.cs | 17 ++ .../Drawables/Steps/RepeatStepButton.cs | 30 ++-- .../Drawables/Steps/SingleStepButton.cs | 19 +-- .../Testing/Drawables/Steps/StepButton.cs | 58 +++---- .../Drawables/Steps/ToggleStepButton.cs | 20 +-- .../Drawables/Steps/UntilStepButton.cs | 70 ++++---- osu.Framework/Testing/TestBrowser.cs | 3 +- osu.Framework/Testing/TestScene.cs | 161 +++++++++--------- 11 files changed, 242 insertions(+), 205 deletions(-) diff --git a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs index 613421f7c7..2b8b7bedac 100644 --- a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs +++ b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs @@ -1,8 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - +using System.Diagnostics; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing.Drawables.Steps; @@ -22,12 +21,42 @@ public TestSceneStepButton() Spacing = new Vector2(5), Children = new Drawable[] { - new LabelStep { Text = nameof(LabelStep) }, - new AssertButton { Text = nameof(AssertButton), Assertion = () => true }, - new SingleStepButton { Text = nameof(SingleStepButton) }, - new RepeatStepButton(null) { Text = nameof(RepeatStepButton) }, - new ToggleStepButton(null) { Text = nameof(ToggleStepButton) }, - new UntilStepButton(() => true) { Text = nameof(UntilStepButton) }, + new LabelStep + { + Text = nameof(LabelStep), + IsSetupStep = false, + Test = this + }, + new AssertButton + { + Text = nameof(AssertButton), + IsSetupStep = false, + Assertion = () => true, + CallStack = new StackTrace() + }, + new SingleStepButton + { + Text = nameof(SingleStepButton), + IsSetupStep = false, + Action = () => { } + }, + new RepeatStepButton + { + Text = nameof(RepeatStepButton), + IsSetupStep = false + }, + new ToggleStepButton + { + Text = nameof(ToggleStepButton), + IsSetupStep = false, + Action = _ => { } + }, + new UntilStepButton + { + Text = nameof(UntilStepButton), + IsSetupStep = false, + Assertion = () => true + }, new StepSlider(nameof(StepSlider), 0, 10, 5), } }; diff --git a/osu.Framework.Tests/Visual/Testing/TestSceneTest.cs b/osu.Framework.Tests/Visual/Testing/TestSceneTest.cs index c93a996bc7..26ba31bf95 100644 --- a/osu.Framework.Tests/Visual/Testing/TestSceneTest.cs +++ b/osu.Framework.Tests/Visual/Testing/TestSceneTest.cs @@ -38,9 +38,10 @@ public virtual void SetUpSteps() if (DebugUtils.IsNUnitRunning && TestContext.CurrentContext.Test.MethodName == nameof(TestConstructor)) return; - AddStep(new SingleStepButton(true) + AddStep(new SingleStepButton { - Name = "set up dummy", + Text = "set up dummy", + IsSetupStep = true, Action = () => setupStepsDummyRun++ }); diff --git a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs index e75e05fb03..16565b1274 100644 --- a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Diagnostics; using System.Text; @@ -13,15 +11,14 @@ namespace osu.Framework.Testing.Drawables.Steps { public partial class AssertButton : StepButton { - public Func Assertion; - public string ExtendedDescription; - public StackTrace CallStack; - private readonly Func getFailureMessage; + public required StackTrace CallStack { get; init; } + public required Func Assertion { get; init; } + public Func? GetFailureMessage { get; init; } + + public string? ExtendedDescription { get; init; } - public AssertButton(bool isSetupStep = false, Func getFailureMessage = null) - : base(isSetupStep) + public AssertButton() { - this.getFailureMessage = getFailureMessage; Action += checkAssert; LightColour = Color4.OrangeRed; } @@ -39,8 +36,8 @@ private void checkAssert() if (!string.IsNullOrEmpty(ExtendedDescription)) builder.Append($" {ExtendedDescription}"); - if (getFailureMessage != null) - builder.Append($": {getFailureMessage()}"); + if (GetFailureMessage != null) + builder.Append($": {GetFailureMessage()}"); throw new TracedException(builder.ToString(), CallStack); } diff --git a/osu.Framework/Testing/Drawables/Steps/LabelStep.cs b/osu.Framework/Testing/Drawables/Steps/LabelStep.cs index c6fcd61616..762c50343a 100644 --- a/osu.Framework/Testing/Drawables/Steps/LabelStep.cs +++ b/osu.Framework/Testing/Drawables/Steps/LabelStep.cs @@ -1,12 +1,18 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; +using osu.Framework.Development; +using osu.Framework.Logging; using osuTK.Graphics; namespace osu.Framework.Testing.Drawables.Steps { public partial class LabelStep : StepButton { + public required TestScene Test { get; init; } + public new Action? Action { get; set; } + protected override Color4 IdleColour => new Color4(77, 77, 77, 255); protected override Color4 RunningColour => new Color4(128, 128, 128, 255); @@ -15,6 +21,17 @@ public LabelStep() { Light.Hide(); Height = 30; + base.Action = clickAction; + } + + private void clickAction() + { + Logger.Log($@"💨 {Test} {Text}"); + + if (!DebugUtils.IsNUnitRunning) + Test.RunAllSteps(startFromStep: this, stopCondition: s => s is LabelStep); + + Action?.Invoke(); } } } diff --git a/osu.Framework/Testing/Drawables/Steps/RepeatStepButton.cs b/osu.Framework/Testing/Drawables/Steps/RepeatStepButton.cs index 5a386b60af..20e7aa4e1f 100644 --- a/osu.Framework/Testing/Drawables/Steps/RepeatStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/RepeatStepButton.cs @@ -1,45 +1,39 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; namespace osu.Framework.Testing.Drawables.Steps { public partial class RepeatStepButton : StepButton { - private readonly int count; - private int invocations; + public int Count { get; init; } = 1; - public override int RequiredRepetitions => count; + public override int RequiredRepetitions => Count; - private string text; + private readonly string text = string.Empty; + private int invocations; - public new string Text + public RepeatStepButton() { - get => text; - set => base.Text = text = value; + updateText(); } - public RepeatStepButton(Action action, int count = 1, bool isSetupStep = false) - : base(isSetupStep) + public new string Text { - this.count = count; - Action = action; - - updateText(); + get => text; + init => base.Text = text = value; } public override void PerformStep(bool userTriggered = false) { - if (invocations == count && !userTriggered) throw new InvalidOperationException("Repeat step was invoked too many times"); + if (invocations == Count && !userTriggered) throw new InvalidOperationException("Repeat step was invoked too many times"); invocations++; base.PerformStep(userTriggered); - if (invocations >= count) // Allows for manual execution beyond the invocation limit. + if (invocations >= Count) // Allows for manual execution beyond the invocation limit. Success(); updateText(); @@ -53,7 +47,7 @@ public override void Reset() updateText(); } - private void updateText() => base.Text = $@"{Text} {invocations}/{count}"; + private void updateText() => base.Text = $@"{Text} {invocations}/{Count}"; public override string ToString() => "Repeat: " + base.ToString(); } diff --git a/osu.Framework/Testing/Drawables/Steps/SingleStepButton.cs b/osu.Framework/Testing/Drawables/Steps/SingleStepButton.cs index bc8806c971..203621e217 100644 --- a/osu.Framework/Testing/Drawables/Steps/SingleStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/SingleStepButton.cs @@ -1,24 +1,23 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; namespace osu.Framework.Testing.Drawables.Steps { public partial class SingleStepButton : StepButton { - public new Action Action; + public new required Action Action { get; init; } + + public SingleStepButton() + { + base.Action = clickAction; + } - public SingleStepButton(bool isSetupStep = false) - : base(isSetupStep) + private void clickAction() { - base.Action = () => - { - Action?.Invoke(); - Success(); - }; + Action(); + Success(); } } } diff --git a/osu.Framework/Testing/Drawables/Steps/StepButton.cs b/osu.Framework/Testing/Drawables/Steps/StepButton.cs index d85b9180ef..23d90f8a2a 100644 --- a/osu.Framework/Testing/Drawables/Steps/StepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/StepButton.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -16,42 +14,20 @@ namespace osu.Framework.Testing.Drawables.Steps { public abstract partial class StepButton : CompositeDrawable { - public virtual int RequiredRepetitions => 1; - - protected Box Light; - protected Box Background; - protected SpriteText SpriteText; - - public Action Action { get; set; } - - public LocalisableString Text - { - get => SpriteText.Text; - set => SpriteText.Text = value; - } - - private Color4 lightColour = Color4.BlueViolet; - - public Color4 LightColour - { - get => lightColour; - set - { - lightColour = value; - if (IsLoaded) Reset(); - } - } + public required bool IsSetupStep { get; init; } + public Action? Action { get; set; } - public readonly bool IsSetupStep; + public virtual int RequiredRepetitions => 1; protected virtual Color4 IdleColour => new Color4(0.15f, 0.15f, 0.15f, 1); - protected virtual Color4 RunningColour => new Color4(0.5f, 0.5f, 0.5f, 1); - protected StepButton(bool isSetupStep = false) - { - IsSetupStep = isSetupStep; + protected readonly Box Light; + protected readonly Box Background; + protected readonly SpriteText SpriteText; + protected StepButton() + { InternalChildren = new Drawable[] { Background = new Box @@ -85,6 +61,24 @@ protected StepButton(bool isSetupStep = false) Masking = true; } + public LocalisableString Text + { + get => SpriteText.Text; + set => SpriteText.Text = value; + } + + private Color4 lightColour = Color4.BlueViolet; + + public Color4 LightColour + { + get => lightColour; + set + { + lightColour = value; + if (IsLoaded) Reset(); + } + } + protected override void LoadComplete() { base.LoadComplete(); diff --git a/osu.Framework/Testing/Drawables/Steps/ToggleStepButton.cs b/osu.Framework/Testing/Drawables/Steps/ToggleStepButton.cs index 5d2b7d8857..41341b6f92 100644 --- a/osu.Framework/Testing/Drawables/Steps/ToggleStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/ToggleStepButton.cs @@ -9,31 +9,31 @@ namespace osu.Framework.Testing.Drawables.Steps { public partial class ToggleStepButton : StepButton { - private readonly Action? reloadCallback; private static readonly Color4 off_colour = Color4.Red; private static readonly Color4 on_colour = Color4.YellowGreen; - public bool State; + public new required Action Action { get; init; } public override int RequiredRepetitions => 2; - public ToggleStepButton(Action? reloadCallback) + private bool state; + + public ToggleStepButton() { - this.reloadCallback = reloadCallback; - Action = clickAction; + base.Action = clickAction; LightColour = off_colour; } private void clickAction() { - State = !State; - Light.FadeColour(State ? on_colour : off_colour); - reloadCallback?.Invoke(State); + state = !state; + Light.FadeColour(state ? on_colour : off_colour); + Action(state); - if (!State) + if (!state) Success(); } - public override string ToString() => $"Toggle: {base.ToString()} ({(State ? "on" : "off")})"; + public override string ToString() => $"Toggle: {base.ToString()} ({(state ? "on" : "off")})"; } } diff --git a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs index 54656d8953..75e63bef91 100644 --- a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Diagnostics; using System.Text; @@ -14,60 +12,58 @@ namespace osu.Framework.Testing.Drawables.Steps { public partial class UntilStepButton : StepButton { - private bool success; - - private int invocations; - private static readonly int max_attempt_milliseconds = FrameworkEnvironment.NoTestTimeout ? int.MaxValue : 10000; + public required Func Assertion { get; init; } + public Func? GetFailureMessage { get; init; } + public new Action? Action { get; set; } + public override int RequiredRepetitions => success ? 0 : int.MaxValue; - public new Action Action; + private readonly string text = string.Empty; + private bool success; + private int invocations; + private Stopwatch? elapsedTime; - private string text; + public UntilStepButton() + { + updateText(); + LightColour = Color4.Sienna; + base.Action = checkAssert; + } public new string Text { get => text; - set => base.Text = text = value; + init => base.Text = text = value; } - private Stopwatch elapsedTime; - - public UntilStepButton(Func waitUntilTrueDelegate, bool isSetupStep = false, Func getFailureMessage = null) - : base(isSetupStep) + private void checkAssert() { + invocations++; + elapsedTime ??= Stopwatch.StartNew(); + updateText(); - LightColour = Color4.Sienna; - base.Action = () => + if (Assertion()) { - invocations++; - - elapsedTime ??= Stopwatch.StartNew(); - - updateText(); - - if (waitUntilTrueDelegate()) - { - elapsedTime = null; - success = true; - Success(); - } - else if (!Debugger.IsAttached && elapsedTime.ElapsedMilliseconds >= max_attempt_milliseconds) - { - StringBuilder builder = new StringBuilder(); + elapsedTime = null; + success = true; + Success(); + } + else if (!Debugger.IsAttached && elapsedTime.ElapsedMilliseconds >= max_attempt_milliseconds) + { + StringBuilder builder = new StringBuilder(); - builder.Append($"\"{Text}\" timed out"); + builder.Append($"\"{Text}\" timed out"); - if (getFailureMessage != null) - builder.Append($": {getFailureMessage()}"); + if (GetFailureMessage != null) + builder.Append($": {GetFailureMessage()}"); - throw new AssertionException(builder.ToString()); - } + throw new AssertionException(builder.ToString()); + } - Action?.Invoke(); - }; + Action?.Invoke(); } public override void Reset() diff --git a/osu.Framework/Testing/TestBrowser.cs b/osu.Framework/Testing/TestBrowser.cs index 3290b01e3f..7ccfca25c6 100644 --- a/osu.Framework/Testing/TestBrowser.cs +++ b/osu.Framework/Testing/TestBrowser.cs @@ -521,9 +521,10 @@ void addSetUpSteps() if (setUpMethods.Any()) { - CurrentTest.AddStep(new SingleStepButton(true) + CurrentTest.AddStep(new SingleStepButton { Text = "[SetUp]", + IsSetupStep = true, LightColour = Color4.Teal, Action = () => setUpMethods.ForEach(s => s.Invoke(CurrentTest, null)) }); diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs index c0dca8ec2f..d7678e1b9d 100644 --- a/osu.Framework/Testing/TestScene.cs +++ b/osu.Framework/Testing/TestScene.cs @@ -271,145 +271,154 @@ private void runNextStep(Action onCompletion, Action onError, Func runNextStep(onCompletion, onError, stopCondition), TimePerAction); } - public void AddStep(StepButton step) => schedule(() => StepsContainer.Add(step)); - - private bool addStepsAsSetupSteps; - public void ChangeBackgroundColour(ColourInfo colour) => backgroundFill.FadeColour(colour, 200, Easing.OutQuint); - public StepButton AddStep(string description, Action action) + private bool addStepsAsSetupSteps; + + public void AddStep(StepButton step) { - var step = new SingleStepButton(addStepsAsSetupSteps) + schedule(() => { - Text = description, - Action = action - }; - - AddStep(step); - - return step; + StepsContainer.Add(step); + }); } - public LabelStep AddLabel(string description) + public void AddStep([NotNull] string description, [NotNull] Action action) { - var step = new LabelStep + AddStep(new SingleStepButton { Text = description, - }; + Action = action, + IsSetupStep = addStepsAsSetupSteps + }); + } - step.Action = () => + public void AddLabel([NotNull] string description) + { + AddStep(new LabelStep { - Logger.Log($@"💨 {this} {description}"); - - // kinda hacky way to avoid this doesn't get triggered by automated runs. - if (step.IsHovered) - RunAllSteps(startFromStep: step, stopCondition: s => s is LabelStep); - }; - - AddStep(step); - - return step; + Text = description, + IsSetupStep = false, + Test = this, + }); } - protected void AddRepeatStep(string description, Action action, int invocationCount) => schedule(() => + protected void AddRepeatStep([NotNull] string description, [NotNull] Action action, int invocationCount) { - StepsContainer.Add(new RepeatStepButton(action, invocationCount, addStepsAsSetupSteps) + AddStep(new RepeatStepButton { Text = description, + IsSetupStep = addStepsAsSetupSteps, + Action = action, + Count = invocationCount }); - }); + } - protected void AddToggleStep(string description, Action action) => schedule(() => + protected void AddToggleStep([NotNull] string description, [NotNull] Action action) { - StepsContainer.Add(new ToggleStepButton(action) + AddStep(new ToggleStepButton { - Text = description + Text = description, + IsSetupStep = addStepsAsSetupSteps, + Action = action, }); - }); + } - protected void AddUntilStep(string description, Func waitUntilTrueDelegate) => schedule(() => + protected void AddUntilStep([CanBeNull] string description, [NotNull] Func waitUntilTrueDelegate) { - StepsContainer.Add(new UntilStepButton(waitUntilTrueDelegate, addStepsAsSetupSteps) + AddStep(new UntilStepButton { Text = description ?? @"Until", + IsSetupStep = addStepsAsSetupSteps, + Assertion = waitUntilTrueDelegate, }); - }); + } - protected void AddUntilStep(string description, ActualValueDelegate actualValue, Func constraint) => schedule(() => + protected void AddUntilStep([CanBeNull] string description, [NotNull] ActualValueDelegate actualValue, [NotNull] Func constraint) { ConstraintResult lastResult = null; - StepsContainer.Add( - new UntilStepButton( - () => - { - lastResult = constraint().Resolve().ApplyTo(actualValue()); - return lastResult.IsSuccess; - }, - addStepsAsSetupSteps, - () => - { - var writer = new TextMessageWriter(string.Empty); - lastResult.WriteMessageTo(writer); - return writer.ToString().TrimStart(); - }) + AddStep(new UntilStepButton + { + Text = description ?? @"Until", + IsSetupStep = addStepsAsSetupSteps, + Assertion = () => { - Text = description ?? @"Until", - }); - }); + lastResult = constraint().Resolve().ApplyTo(actualValue()); + return lastResult.IsSuccess; + }, + GetFailureMessage = () => + { + if (lastResult == null) + return string.Empty; + + var writer = new TextMessageWriter(string.Empty); + lastResult.WriteMessageTo(writer); + return writer.ToString().TrimStart(); + } + }); + } - protected void AddWaitStep(string description, int waitCount) => schedule(() => + protected void AddWaitStep([CanBeNull] string description, int waitCount) { - StepsContainer.Add(new RepeatStepButton(() => { }, waitCount, addStepsAsSetupSteps) + AddStep(new RepeatStepButton { Text = description ?? @"Wait", + IsSetupStep = addStepsAsSetupSteps, + Count = waitCount }); - }); + } - protected void AddSliderStep(string description, T min, T max, T start, Action valueChanged) where T : struct, INumber, IMinMaxValue => schedule(() => + protected void AddSliderStep([NotNull] string description, T min, T max, T start, [NotNull] Action valueChanged) where T : struct, INumber, IMinMaxValue { - StepsContainer.Add(new StepSlider(description, min, max, start) + schedule(() => { - ValueChanged = valueChanged, + StepsContainer.Add(new StepSlider(description, min, max, start) + { + ValueChanged = valueChanged, + }); }); - }); + } - protected void AddAssert(string description, Func assert, string extendedDescription = null) => schedule(() => + protected void AddAssert([NotNull] string description, [NotNull] Func assert, [CanBeNull] string extendedDescription = null) { - StepsContainer.Add(new AssertButton(addStepsAsSetupSteps) + AddStep(new AssertButton { Text = description, + IsSetupStep = addStepsAsSetupSteps, ExtendedDescription = extendedDescription, CallStack = new StackTrace(1), Assertion = assert, }); - }); + } - protected void AddAssert(string description, ActualValueDelegate actualValue, Func constraint, string extendedDescription = null) => schedule(() => + protected void AddAssert([NotNull] string description, [NotNull] ActualValueDelegate actualValue, [NotNull] Func constraint, [CanBeNull] string extendedDescription = null) { ConstraintResult lastResult = null; - StepsContainer.Add(new AssertButton(addStepsAsSetupSteps, () => - { - if (lastResult == null) - return string.Empty; - - var writer = new TextMessageWriter(string.Empty); - lastResult.WriteMessageTo(writer); - return writer.ToString().TrimStart(); - }) + AddStep(new AssertButton { Text = description, + IsSetupStep = addStepsAsSetupSteps, ExtendedDescription = extendedDescription, CallStack = new StackTrace(1), Assertion = () => { lastResult = constraint().Resolve().ApplyTo(actualValue()); return lastResult.IsSuccess; + }, + GetFailureMessage = () => + { + if (lastResult == null) + return string.Empty; + + var writer = new TextMessageWriter(string.Empty); + lastResult.WriteMessageTo(writer); + return writer.ToString().TrimStart(); } }); - }); + } internal void RunSetUpSteps() { From 42a01b939964d1f12a60f8f866fa635ed49fcfc7 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 11 Nov 2024 23:50:11 +0900 Subject: [PATCH 14/26] Give UntilStepButton a proper stacktrace --- .../Visual/Testing/TestSceneStepButton.cs | 3 ++- .../Testing/Drawables/Steps/AssertButton.cs | 14 ------------- .../Drawables/Steps/TracedException.cs | 21 +++++++++++++++++++ .../Drawables/Steps/UntilStepButton.cs | 4 ++-- osu.Framework/Testing/TestScene.cs | 2 ++ 5 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 osu.Framework/Testing/Drawables/Steps/TracedException.cs diff --git a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs index 2b8b7bedac..193cc4adc6 100644 --- a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs +++ b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs @@ -55,7 +55,8 @@ public TestSceneStepButton() { Text = nameof(UntilStepButton), IsSetupStep = false, - Assertion = () => true + Assertion = () => true, + CallStack = new StackTrace() }, new StepSlider(nameof(StepSlider), 0, 10, 5), } diff --git a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs index 16565b1274..a57d2eb628 100644 --- a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.Text; -using NUnit.Framework; using osuTK.Graphics; namespace osu.Framework.Testing.Drawables.Steps @@ -44,18 +43,5 @@ private void checkAssert() } public override string ToString() => "Assert: " + base.ToString(); - - private class TracedException : AssertionException - { - private readonly StackTrace trace; - - public TracedException(string description, StackTrace trace) - : base(description) - { - this.trace = trace; - } - - public override string StackTrace => trace.ToString(); - } } } diff --git a/osu.Framework/Testing/Drawables/Steps/TracedException.cs b/osu.Framework/Testing/Drawables/Steps/TracedException.cs new file mode 100644 index 0000000000..bd2284e96b --- /dev/null +++ b/osu.Framework/Testing/Drawables/Steps/TracedException.cs @@ -0,0 +1,21 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Diagnostics; +using NUnit.Framework; + +namespace osu.Framework.Testing.Drawables.Steps +{ + internal class TracedException : AssertionException + { + private readonly StackTrace trace; + + public TracedException(string description, StackTrace trace) + : base(description) + { + this.trace = trace; + } + + public override string StackTrace => trace.ToString(); + } +} diff --git a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs index 75e63bef91..b3c6bd6722 100644 --- a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.Text; -using NUnit.Framework; using osu.Framework.Graphics; using osuTK.Graphics; @@ -14,6 +13,7 @@ public partial class UntilStepButton : StepButton { private static readonly int max_attempt_milliseconds = FrameworkEnvironment.NoTestTimeout ? int.MaxValue : 10000; + public required StackTrace CallStack { get; init; } public required Func Assertion { get; init; } public Func? GetFailureMessage { get; init; } public new Action? Action { get; set; } @@ -60,7 +60,7 @@ private void checkAssert() if (GetFailureMessage != null) builder.Append($": {GetFailureMessage()}"); - throw new AssertionException(builder.ToString()); + throw new TracedException(builder.ToString(), CallStack); } Action?.Invoke(); diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs index d7678e1b9d..9bdf41f8ba 100644 --- a/osu.Framework/Testing/TestScene.cs +++ b/osu.Framework/Testing/TestScene.cs @@ -331,6 +331,7 @@ protected void AddUntilStep([CanBeNull] string description, [NotNull] Func { Text = description ?? @"Until", IsSetupStep = addStepsAsSetupSteps, + CallStack = new StackTrace(1), Assertion = waitUntilTrueDelegate, }); } @@ -343,6 +344,7 @@ protected void AddUntilStep([CanBeNull] string description, [NotNull] ActualV { Text = description ?? @"Until", IsSetupStep = addStepsAsSetupSteps, + CallStack = new StackTrace(1), Assertion = () => { lastResult = constraint().Resolve().ApplyTo(actualValue()); From 3960655a4286d69038857413b3d47c3fef4bb813 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 11 Nov 2024 23:50:50 +0900 Subject: [PATCH 15/26] Further improve stacktrace output --- .../Testing/Drawables/Steps/AssertButton.cs | 4 +++- .../Drawables/Steps/TracedException.cs | 21 ------------------- .../Drawables/Steps/UntilStepButton.cs | 4 +++- osu.Framework/Testing/TestScene.cs | 11 +++++----- 4 files changed, 12 insertions(+), 28 deletions(-) delete mode 100644 osu.Framework/Testing/Drawables/Steps/TracedException.cs diff --git a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs index a57d2eb628..d910acb964 100644 --- a/osu.Framework/Testing/Drawables/Steps/AssertButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/AssertButton.cs @@ -3,7 +3,9 @@ using System; using System.Diagnostics; +using System.Runtime.ExceptionServices; using System.Text; +using NUnit.Framework; using osuTK.Graphics; namespace osu.Framework.Testing.Drawables.Steps @@ -38,7 +40,7 @@ private void checkAssert() if (GetFailureMessage != null) builder.Append($": {GetFailureMessage()}"); - throw new TracedException(builder.ToString(), CallStack); + throw ExceptionDispatchInfo.SetRemoteStackTrace(new AssertionException(builder.ToString()), CallStack.ToString()); } } diff --git a/osu.Framework/Testing/Drawables/Steps/TracedException.cs b/osu.Framework/Testing/Drawables/Steps/TracedException.cs deleted file mode 100644 index bd2284e96b..0000000000 --- a/osu.Framework/Testing/Drawables/Steps/TracedException.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Diagnostics; -using NUnit.Framework; - -namespace osu.Framework.Testing.Drawables.Steps -{ - internal class TracedException : AssertionException - { - private readonly StackTrace trace; - - public TracedException(string description, StackTrace trace) - : base(description) - { - this.trace = trace; - } - - public override string StackTrace => trace.ToString(); - } -} diff --git a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs index b3c6bd6722..89447a9512 100644 --- a/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs +++ b/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs @@ -3,7 +3,9 @@ using System; using System.Diagnostics; +using System.Runtime.ExceptionServices; using System.Text; +using NUnit.Framework; using osu.Framework.Graphics; using osuTK.Graphics; @@ -60,7 +62,7 @@ private void checkAssert() if (GetFailureMessage != null) builder.Append($": {GetFailureMessage()}"); - throw new TracedException(builder.ToString(), CallStack); + throw ExceptionDispatchInfo.SetRemoteStackTrace(new AssertionException(builder.ToString()), CallStack.ToString()); } Action?.Invoke(); diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs index 9bdf41f8ba..c781cd9619 100644 --- a/osu.Framework/Testing/TestScene.cs +++ b/osu.Framework/Testing/TestScene.cs @@ -331,7 +331,7 @@ protected void AddUntilStep([CanBeNull] string description, [NotNull] Func { Text = description ?? @"Until", IsSetupStep = addStepsAsSetupSteps, - CallStack = new StackTrace(1), + CallStack = new StackTrace(1, true), Assertion = waitUntilTrueDelegate, }); } @@ -344,7 +344,7 @@ protected void AddUntilStep([CanBeNull] string description, [NotNull] ActualV { Text = description ?? @"Until", IsSetupStep = addStepsAsSetupSteps, - CallStack = new StackTrace(1), + CallStack = new StackTrace(1, true), Assertion = () => { lastResult = constraint().Resolve().ApplyTo(actualValue()); @@ -390,12 +390,13 @@ protected void AddAssert([NotNull] string description, [NotNull] Func asse Text = description, IsSetupStep = addStepsAsSetupSteps, ExtendedDescription = extendedDescription, - CallStack = new StackTrace(1), + CallStack = new StackTrace(1, true), Assertion = assert, }); } - protected void AddAssert([NotNull] string description, [NotNull] ActualValueDelegate actualValue, [NotNull] Func constraint, [CanBeNull] string extendedDescription = null) + protected void AddAssert([NotNull] string description, [NotNull] ActualValueDelegate actualValue, [NotNull] Func constraint, + [CanBeNull] string extendedDescription = null) { ConstraintResult lastResult = null; @@ -404,7 +405,7 @@ protected void AddAssert([NotNull] string description, [NotNull] ActualValueD Text = description, IsSetupStep = addStepsAsSetupSteps, ExtendedDescription = extendedDescription, - CallStack = new StackTrace(1), + CallStack = new StackTrace(1, true), Assertion = () => { lastResult = constraint().Resolve().ApplyTo(actualValue()); From 43fdea246d371e897800e2f8f7d5f4d7222b7852 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 12 Nov 2024 01:06:22 +0900 Subject: [PATCH 16/26] Restore LabelStep implementation --- .../Visual/Testing/TestSceneStepButton.cs | 2 +- .../Testing/Drawables/Steps/LabelStep.cs | 15 ++------------- osu.Framework/Testing/TestScene.cs | 9 ++++++++- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs index 193cc4adc6..602709ffc2 100644 --- a/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs +++ b/osu.Framework.Tests/Visual/Testing/TestSceneStepButton.cs @@ -25,7 +25,7 @@ public TestSceneStepButton() { Text = nameof(LabelStep), IsSetupStep = false, - Test = this + Action = _ => { }, }, new AssertButton { diff --git a/osu.Framework/Testing/Drawables/Steps/LabelStep.cs b/osu.Framework/Testing/Drawables/Steps/LabelStep.cs index 762c50343a..5f1271a63a 100644 --- a/osu.Framework/Testing/Drawables/Steps/LabelStep.cs +++ b/osu.Framework/Testing/Drawables/Steps/LabelStep.cs @@ -2,16 +2,13 @@ // See the LICENCE file in the repository root for full licence text. using System; -using osu.Framework.Development; -using osu.Framework.Logging; using osuTK.Graphics; namespace osu.Framework.Testing.Drawables.Steps { public partial class LabelStep : StepButton { - public required TestScene Test { get; init; } - public new Action? Action { get; set; } + public new required Action Action { get; init; } protected override Color4 IdleColour => new Color4(77, 77, 77, 255); @@ -24,14 +21,6 @@ public LabelStep() base.Action = clickAction; } - private void clickAction() - { - Logger.Log($@"💨 {Test} {Text}"); - - if (!DebugUtils.IsNUnitRunning) - Test.RunAllSteps(startFromStep: this, stopCondition: s => s is LabelStep); - - Action?.Invoke(); - } + private void clickAction() => Action(this); } } diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs index c781cd9619..4cc97bf25f 100644 --- a/osu.Framework/Testing/TestScene.cs +++ b/osu.Framework/Testing/TestScene.cs @@ -300,7 +300,14 @@ public void AddLabel([NotNull] string description) { Text = description, IsSetupStep = false, - Test = this, + Action = step => + { + Logger.Log($@"💨 {this} {description}"); + + // kinda hacky way to avoid this doesn't get triggered by automated runs. + if (step.IsHovered) + RunAllSteps(startFromStep: step, stopCondition: s => s is LabelStep); + }, }); } From d9603f72f305843bb0fafd17a41bcdcd7239ad9d Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 11 Nov 2024 16:53:59 +0000 Subject: [PATCH 17/26] Use SDL3 text editing events on windows `StartTextInput()` and `ResetIme()` are kept as they improve and can co-exist with SDL3 logic. --- .../Platform/Windows/SDL3WindowsWindow.cs | 111 ------------------ 1 file changed, 111 deletions(-) diff --git a/osu.Framework/Platform/Windows/SDL3WindowsWindow.cs b/osu.Framework/Platform/Windows/SDL3WindowsWindow.cs index 7c065b98f2..d04ea28ba1 100644 --- a/osu.Framework/Platform/Windows/SDL3WindowsWindow.cs +++ b/osu.Framework/Platform/Windows/SDL3WindowsWindow.cs @@ -2,12 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Diagnostics; using System.Drawing; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; -using osu.Framework.Allocation; using osu.Framework.Input.Handlers.Mouse; using osu.Framework.Platform.SDL3; using osu.Framework.Platform.Windows.Native; @@ -62,36 +59,6 @@ public override void Create() Native.Input.SetWindowFeedbackSetting(WindowHandle, feedbackType, false); } - public override unsafe void Run() - { - SDL_SetWindowsMessageHook(&messageHook, ObjectHandle.Handle); - base.Run(); - } - - [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] - private static unsafe SDLBool messageHook(IntPtr userdata, MSG* msg) - { - var handle = new ObjectHandle(userdata); - if (handle.GetTarget(out SDL3WindowsWindow window)) - return window.handleEventFromHook(*msg); - - return true; - } - - private SDLBool handleEventFromHook(MSG msg) - { - switch (msg.message) - { - case Imm.WM_IME_STARTCOMPOSITION: - case Imm.WM_IME_COMPOSITION: - case Imm.WM_IME_ENDCOMPOSITION: - handleImeMessage(msg.hwnd, msg.message, msg.lParam); - break; - } - - return true; - } - protected override void HandleEventFromFilter(SDL_Event evt) { switch (evt.Type) @@ -125,8 +92,6 @@ private void warpCursorFromFocusLoss() } } - #region IME handling - public override void StartTextInput(bool allowIme) { base.StartTextInput(allowIme); @@ -135,82 +100,6 @@ public override void StartTextInput(bool allowIme) public override void ResetIme() => ScheduleCommand(() => Imm.CancelComposition(WindowHandle)); - protected override void HandleTextInputEvent(SDL_TextInputEvent evtText) - { - string? sdlResult = evtText.GetText(); - Debug.Assert(sdlResult != null); - - // Block SDL text input if it was already handled by `handleImeMessage()`. - // SDL truncates text over 32 bytes and sends it as multiple events. - // We assume these events will be handled in the same `pollSDLEvents()` call. - if (lastImeResult?.Contains(sdlResult) == true) - { - // clear the result after this SDL event loop finishes so normal text input isn't blocked. - EventScheduler.AddOnce(() => lastImeResult = null); - return; - } - - // also block if there is an ongoing composition (unlikely to occur). - if (imeCompositionActive) return; - - base.HandleTextInputEvent(evtText); - } - - protected override void HandleTextEditingEvent(SDL_TextEditingEvent evtEdit) - { - // handled by custom logic below - } - - /// - /// Whether IME composition is active. - /// - /// Used for blocking SDL IME results since we handle those ourselves. - private bool imeCompositionActive; - - /// - /// The last IME result. - /// - /// - /// Used for blocking SDL IME results since we handle those ourselves. - /// Cleared when the SDL events are blocked. - /// - private string? lastImeResult; - - private void handleImeMessage(IntPtr hWnd, uint uMsg, long lParam) - { - switch (uMsg) - { - case Imm.WM_IME_STARTCOMPOSITION: - imeCompositionActive = true; - ScheduleEvent(() => TriggerTextEditing(string.Empty, 0, 0)); - break; - - case Imm.WM_IME_COMPOSITION: - using (var inputContext = new Imm.InputContext(hWnd, lParam)) - { - if (inputContext.TryGetImeResult(out string? resultText)) - { - lastImeResult = resultText; - ScheduleEvent(() => TriggerTextInput(resultText)); - } - - if (inputContext.TryGetImeComposition(out string? compositionText, out int start, out int length)) - { - ScheduleEvent(() => TriggerTextEditing(compositionText, start, length)); - } - } - - break; - - case Imm.WM_IME_ENDCOMPOSITION: - imeCompositionActive = false; - ScheduleEvent(() => TriggerTextEditing(string.Empty, 0, 0)); - break; - } - } - - #endregion - protected override void HandleTouchFingerEvent(SDL_TouchFingerEvent evtTfinger) { if (evtTfinger.TryGetTouchName(out string? name) && name == "pen") From 15c612fe868c72aa8ef31bdd97b01d918ecae733 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 11 Nov 2024 16:54:05 +0000 Subject: [PATCH 18/26] Remove unnecessary virtual & protected modifiers --- osu.Framework/Platform/SDL3/SDL3Window.cs | 4 ++-- osu.Framework/Platform/SDL3/SDL3Window_Input.cs | 12 ++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/osu.Framework/Platform/SDL3/SDL3Window.cs b/osu.Framework/Platform/SDL3/SDL3Window.cs index 5944581b10..77c0f4fc3f 100644 --- a/osu.Framework/Platform/SDL3/SDL3Window.cs +++ b/osu.Framework/Platform/SDL3/SDL3Window.cs @@ -501,11 +501,11 @@ protected virtual void HandleEvent(SDL_Event e) break; case SDL_EventType.SDL_EVENT_TEXT_EDITING: - HandleTextEditingEvent(e.edit); + handleTextEditingEvent(e.edit); break; case SDL_EventType.SDL_EVENT_TEXT_INPUT: - HandleTextInputEvent(e.text); + handleTextInputEvent(e.text); break; case SDL_EventType.SDL_EVENT_KEYMAP_CHANGED: diff --git a/osu.Framework/Platform/SDL3/SDL3Window_Input.cs b/osu.Framework/Platform/SDL3/SDL3Window_Input.cs index 5c340f8e37..7709f531eb 100644 --- a/osu.Framework/Platform/SDL3/SDL3Window_Input.cs +++ b/osu.Framework/Platform/SDL3/SDL3Window_Input.cs @@ -472,18 +472,18 @@ private void handleMouseMotionEvent(SDL_MouseMotionEvent evtMotion) MouseMoveRelative?.Invoke(new Vector2(evtMotion.xrel * Scale, evtMotion.yrel * Scale)); } - protected virtual void HandleTextInputEvent(SDL_TextInputEvent evtText) + private void handleTextInputEvent(SDL_TextInputEvent evtText) { string? text = evtText.GetText(); Debug.Assert(text != null); - TriggerTextInput(text); + TextInput?.Invoke(text); } - protected virtual void HandleTextEditingEvent(SDL_TextEditingEvent evtEdit) + private void handleTextEditingEvent(SDL_TextEditingEvent evtEdit) { string? text = evtEdit.GetText(); Debug.Assert(text != null); - TriggerTextEditing(text, evtEdit.start, evtEdit.length); + TextEditing?.Invoke(text, evtEdit.start, evtEdit.length); } private void handleKeyboardEvent(SDL_KeyboardEvent evtKey) @@ -713,15 +713,11 @@ private void updateConfineMode() /// public event Action? TextInput; - protected void TriggerTextInput(string text) => TextInput?.Invoke(text); - /// /// Invoked when an IME text editing event occurs. /// public event TextEditingDelegate? TextEditing; - protected void TriggerTextEditing(string text, int start, int length) => TextEditing?.Invoke(text, start, length); - /// public event Action? KeymapChanged; From 6b849a96c2c8bf683e575e75ed881491cab75d84 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 12 Nov 2024 02:18:32 +0900 Subject: [PATCH 19/26] Make DrawVisualiser follow proxied drawables to their visual layer --- osu.Framework/Graphics/Visualisation/DrawVisualiser.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs b/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs index 2362c7c73e..df0a4810fb 100644 --- a/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs +++ b/osu.Framework/Graphics/Visualisation/DrawVisualiser.cs @@ -222,6 +222,14 @@ private void updateCursorTarget() // Finds the targeted drawable and composite drawable. The search stops if a drawable is targeted. void findTarget(Drawable drawable) { + // Ignore proxied drawables (they may be at a different visual layer). + if (drawable.HasProxy) + return; + + // When a proxy is encountered, restore the original drawable for target testing. + while (drawable.IsProxy) + drawable = drawable.Original; + if (drawable == this || drawable is Component) return; From a73f205b639c85dfd3f59df7f7ca4475d59b5d72 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 11 Nov 2024 16:38:30 -0500 Subject: [PATCH 20/26] Fix error triggered in label step execution not logged --- osu.Framework/Testing/TestBrowser.cs | 2 +- osu.Framework/Testing/TestScene.cs | 8 ++++---- osu.Framework/Testing/TestSceneTestRunner.cs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Framework/Testing/TestBrowser.cs b/osu.Framework/Testing/TestBrowser.cs index 7ccfca25c6..8f5b708471 100644 --- a/osu.Framework/Testing/TestBrowser.cs +++ b/osu.Framework/Testing/TestBrowser.cs @@ -590,7 +590,7 @@ private static IEnumerable getTestCaseSourceValue(MethodInfo testMethod, TestCas private void runTests(Action onCompletion) { int actualStepCount = 0; - CurrentTest.RunAllSteps(onCompletion, e => Logger.Log($@"Error on step: {e}"), s => + CurrentTest.RunAllSteps(onCompletion, (s, e) => Logger.Error(e, $"Step {s} triggered an error"), s => { if (!interactive || RunAllSteps.Value) return false; diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs index 4cc97bf25f..545f59161e 100644 --- a/osu.Framework/Testing/TestScene.cs +++ b/osu.Framework/Testing/TestScene.cs @@ -204,7 +204,7 @@ protected TestScene() private ScheduledDelegate stepRunner; private readonly ScrollContainer scroll; - public void RunAllSteps(Action onCompletion = null, Action onError = null, Func stopCondition = null, StepButton startFromStep = null) + public void RunAllSteps(Action onCompletion = null, Action onError = null, Func stopCondition = null, StepButton startFromStep = null) { // schedule once as we want to ensure we have run our LoadComplete before attempting to execute steps. // a user may be adding a step in LoadComplete. @@ -222,7 +222,7 @@ public void RunAllSteps(Action onCompletion = null, Action onError = private StepButton loadableStep => actionIndex >= 0 ? StepsContainer.Children.ElementAtOrDefault(actionIndex) as StepButton : null; - private void runNextStep(Action onCompletion, Action onError, Func stopCondition) + private void runNextStep(Action onCompletion, Action onError, Func stopCondition) { try { @@ -242,7 +242,7 @@ private void runNextStep(Action onCompletion, Action onError, Func s is LabelStep); + RunAllSteps(startFromStep: step, stopCondition: s => s is LabelStep, onError: (s, e) => Logger.Error(e, $"Step {s} triggered error")); }, }); } diff --git a/osu.Framework/Testing/TestSceneTestRunner.cs b/osu.Framework/Testing/TestSceneTestRunner.cs index f4e6bf53fc..32f05b7076 100644 --- a/osu.Framework/Testing/TestSceneTestRunner.cs +++ b/osu.Framework/Testing/TestSceneTestRunner.cs @@ -76,7 +76,7 @@ void complete() test.RunAllSteps(() => { Scheduler.AddDelayed(complete, time_between_tests); - }, e => + }, (_, e) => { exception = ExceptionDispatchInfo.Capture(e); complete(); From 0ec84c037eb7fdd7488cafac0d014a0b18888fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 12 Nov 2024 11:35:44 +0100 Subject: [PATCH 21/26] Fix CI test report workflow Carbon copy of https://github.com/ppy/osu/pull/30543. --- .github/workflows/report-nunit.yml | 34 +++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/.github/workflows/report-nunit.yml b/.github/workflows/report-nunit.yml index a26723d84f..476000a223 100644 --- a/.github/workflows/report-nunit.yml +++ b/.github/workflows/report-nunit.yml @@ -5,30 +5,40 @@ name: Annotate CI run with test results on: workflow_run: - workflows: ["Continuous Integration"] + workflows: [ "Continuous Integration" ] types: - completed + +permissions: + contents: read + actions: read + checks: write + jobs: annotate: name: Annotate CI run with test results runs-on: ubuntu-latest if: ${{ github.event.workflow_run.conclusion != 'cancelled' }} - strategy: - fail-fast: false - matrix: - os: - - { prettyname: Windows, configuration: Debug } - - { prettyname: macOS, configuration: Debug } - - { prettyname: Linux, configuration: Debug } - - { prettyname: Linux, configuration: Release } - threadingMode: ['SingleThread', 'MultiThreaded'] timeout-minutes: 5 steps: + - name: Checkout + uses: actions/checkout@v4 + with: + repository: ${{ github.event.workflow_run.repository.full_name }} + ref: ${{ github.event.workflow_run.head_sha }} + + - name: Download results + uses: actions/download-artifact@v4 + with: + pattern: osu-framework-test-results-* + merge-multiple: true + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + - name: Annotate CI run with test results uses: dorny/test-reporter@v1.8.0 with: - artifact: osu-framework-test-results-${{matrix.os.prettyname}}-${{matrix.threadingMode}}-${{matrix.os.configuration}} - name: Test Results (${{matrix.os.prettyname}}, ${{matrix.threadingMode}}, ${{matrix.os.configuration}}) + name: Results path: "*.trx" reporter: dotnet-trx list-suites: 'failed' From 9e385850d7115f9854f6a666a954a38df4e45b79 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 14 Nov 2024 19:29:53 +0900 Subject: [PATCH 22/26] Add failing test --- .../TestSceneRearrangeableListContainer.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs index 83450a664e..c4397a8c50 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Collections.Generic; using System.Linq; @@ -21,9 +19,8 @@ namespace osu.Framework.Tests.Visual.UserInterface { public partial class TestSceneRearrangeableListContainer : ManualInputManagerTestScene { - private TestRearrangeableList list; - - private Container listContainer; + private TestRearrangeableList list = null!; + private Container listContainer = null!; [SetUp] public void Setup() => Schedule(() => @@ -85,7 +82,7 @@ public void TestRemoveItem() addItems(item_count); - List items = null; + List items = null!; AddStep("get item references", () => items = new List(list.ItemMap.Values.ToList())); @@ -278,7 +275,7 @@ public void TestNotScrolledToTopOnRemove() [Test] public void TestRemoveDuringLoadAndReAdd() { - TestDelayedLoadRearrangeableList delayedList = null; + TestDelayedLoadRearrangeableList delayedList = null!; AddStep("create list", () => Child = delayedList = new TestDelayedLoadRearrangeableList()); @@ -327,6 +324,15 @@ public void TestDragSynchronisation() }); } + [Test] + public void TestReplaceEntireList() + { + addItems(1); + + AddStep("replace list", () => list.Items.ReplaceRange(0, list.Items.Count, [100])); + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + } + private void addDragSteps(int from, int to, int[] expectedSequence) { AddStep($"move to {from}", () => From 23443bfd26ef710da0f96dbd75cecdc07dc593c5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 14 Nov 2024 19:57:37 +0900 Subject: [PATCH 23/26] Fix `RearrangeableListContainer<>` crashing on replacement operations --- .../Containers/RearrangeableListContainer.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs index 1363206638..b133534c31 100644 --- a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs +++ b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs @@ -183,11 +183,15 @@ private void sortItems() { for (int i = 0; i < Items.Count; i++) { - var drawable = itemMap[Items[i]]; + // A drawable for the item may not exist yet, for example in a replace-range operation where the removal happens first. + if (!itemMap.TryGetValue(Items[i], out var drawable)) + continue; + + // The item may not be loaded yet, because add operations are asynchronous. + if (drawable.Parent != ListContainer) + continue; - // If the async load didn't complete, the item wouldn't exist in the container and an exception would be thrown - if (drawable.Parent == ListContainer) - ListContainer!.SetLayoutPosition(drawable, i); + ListContainer!.SetLayoutPosition(drawable, i); } } From 69d1f7f5b8a88839121353cfb0ee3d705cf30d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 14 Nov 2024 13:43:17 +0100 Subject: [PATCH 24/26] Add another test for partial replace operation For no particular reason other than to see if it does the right thing, and it does. --- .../UserInterface/TestSceneRearrangeableListContainer.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs index c4397a8c50..9c3ede1100 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs @@ -333,6 +333,15 @@ public void TestReplaceEntireList() AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); } + [Test] + public void TestPartialReplace() + { + addItems(5); + + AddStep("replace list", () => list.Items.ReplaceRange(2, 2, [100, 101])); + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + } + private void addDragSteps(int from, int to, int[] expectedSequence) { AddStep($"move to {from}", () => From 9b9ec67e207cb1ce64cb11937e6e86fc9d1df0cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 15 Nov 2024 13:38:04 +0100 Subject: [PATCH 25/26] Revert "Use SDL3 windowing by default" --- osu.Framework/FrameworkEnvironment.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/FrameworkEnvironment.cs b/osu.Framework/FrameworkEnvironment.cs index f5c4369bbc..5b4b5ee8ee 100644 --- a/osu.Framework/FrameworkEnvironment.cs +++ b/osu.Framework/FrameworkEnvironment.cs @@ -53,7 +53,7 @@ static FrameworkEnvironment() if (DebugUtils.IsDebugBuild) AllowInsecureRequests = parseBool(Environment.GetEnvironmentVariable("OSU_INSECURE_REQUESTS")) ?? false; - UseSDL3 = RuntimeInfo.IsMobile || (parseBool(Environment.GetEnvironmentVariable("OSU_SDL3")) ?? true); + UseSDL3 = RuntimeInfo.IsMobile || (parseBool(Environment.GetEnvironmentVariable("OSU_SDL3")) ?? false); } private static bool? parseBool(string? value) From d694ce805265fa058f2d887f1a3d2d7d47433cfe Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Sat, 16 Nov 2024 12:57:34 +0900 Subject: [PATCH 26/26] Remove `NuGet.ProjectModel` dependency --- osu.Framework/osu.Framework.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Framework/osu.Framework.csproj b/osu.Framework/osu.Framework.csproj index f9b558c41b..ca76688201 100644 --- a/osu.Framework/osu.Framework.csproj +++ b/osu.Framework/osu.Framework.csproj @@ -23,7 +23,6 @@ -