Avalonia UI by Deterous · Pull Request #975 · SabreTools/MPF

Deterous · 2026-05-06T01:07:35Z

This PR adds an alternate UI framework for MPF based on Avalonia, allowing for Linux and macOS UI builds.
There may be some small tweaks needed before merging during my testing, but I want to open the PR now to begin the review process as this is a big change.
Thanks to 7 (@whatev-indus) for putting in the work on this, I've just helped with guidance and then cleaned it up to use SabreTools styles and build processes. I will also be responding to any PR review comments. If you want to make changes to the branch directly and cannot, ask 7 for contributor access to the repo.

This does not replace the WPF UI, currently both are built separately. The images (icon, ringcode pics) are referenced from the WPF folder, but the Strings XAML are duplicated. The new MPF.Avalonia namespace is intended to reference the MPF.Frontend namespace as much as possible, so that it can be a drop-in replacement for the old WPF UI.

It is also 7's request to have AppImage builds for Linux and other build changes required for icons and niceties on macOS, although these changes can come in a later PR (to minimize the review burden)

This reverts commit 0ce6c7d.

mnadareski

Many of the inline comments pertain to multiple locations, but there were a couple of overarching comments that I wanted to mention as well:

There are basically no code comments. For anything that was based on existing WPF code, please copy the comment wholesale. For anything that is new functionality, add at the bare minimum a summary statement about what it is doing. I know that many are self-explanatory once you read the method, but those comments are used to get an understanding while coding to avoid having to look at the source for all details.
All loops, such as for, foreach, and while should have curly braces, regardless of the number of lines that are contained within.
I am seeing the use of global:: in many places and I would like to understand more about why this pattern is being used.
Unless there is ambiguity about what method or property is being invoked, you can avoid use of the this. qualifier.
There are many places, such as large lists of constants/variables or OS-specific methods, that could do with #region tags. This can help break up the code into more distinct sections and allows collapsing away logical groupings.

mnadareski · 2026-05-28T03:55:10Z

+        /// <summary>
+        /// UI-friendly display name for the drive
+        /// </summary>
+        public string DisplayName => GetDisplayName();


This can be combined with the helper method into a cached property that utilizes the field keyword.

mnadareski · 2026-05-28T03:56:07Z

                DriveFormat = driveInfo.DriveFormat;
                TotalSize = driveInfo.TotalSize;
-                VolumeLabel = driveInfo.VolumeLabel;
+                if (Environment.OSVersion.Platform != PlatformID.Unix)


If this is specific to Windows, then I'd prefer the check to say that instead. If saying "non-Unix" is actually the right way of doing something like Windows and Mac only, I would appreciate a one-line comment above this if/else that says that.

You're right that this is effectively Windows-only. DriveInfo.VolumeLabel is only available on Windows, and modern .NET reports both Linux and macOS as PlatformID.Unix, so != PlatformID.Unix resolves to "Windows" here. I didn't switch to OperatingSystem.IsWindows() because this project also targets net20-net48, where that helper doesn't exist. I've added a comment above the if/else explaining exactly that (commit 548f0ff).

Updated to make the check explicit (commit b76b4cb): switched != PlatformID.Unix to == PlatformID.Win32NT, which reads as "Windows" directly as you preferred. I used PlatformID.Win32NT rather than OperatingSystem.IsWindows() because that helper only exists on net5.0+, and this project also targets net20-net48; PlatformID.Win32NT is available on all of them. The comment is now a single line noting that DriveInfo.VolumeLabel is Windows-only.

mnadareski · 2026-05-28T03:57:46Z

+            if (displayName.Length > 1)
+                displayName = displayName.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);


I don't understand why this has to be gated by a length check

mnadareski · 2026-05-28T13:13:39Z

+                "L0MasteringRing", "L0MasteringSID", "L0Toolstamp", "L0MouldSID", "L0AdditionalMould",
+                "L1MasteringRing", "L1MasteringSID", "L1Toolstamp", "L1MouldSID", "L1AdditionalMould",
+                "L2MasteringRing", "L2MasteringSID", "L2Toolstamp",
+                "L3MasteringRing", "L3MasteringSID", "L3Toolstamp",


This feels like it should be a static readonly collection at the class level. I have been thrown off seeing this in the declaration of a foreach statement multiple times now.

Done (commit 536e1ee) — extracted to static readonly class-level arrays so they're no longer declared inline in the foreach: TabEnabledFieldNames here, and LanguageMenuItemNames for the equivalent block in MainWindow.

mnadareski · 2026-05-28T13:18:42Z

+    {
+        private readonly bool _showPcMacHybridAlways;
+
+        private static readonly (string Name, SiteCode Code)[] CommentFields =


Why are these arrays of tuples instead of a dictionary?

They're kept as ordered arrays because display order matters — the comment/content fields render in a fixed sequence, and a Dictionary doesn't guarantee enumeration order, so switching would risk shuffling the UI. They're also small, fixed lists iterated linearly, so a dictionary's lookup benefit doesn't really apply here. If keyed lookup is needed elsewhere I could add a dictionary alongside, but the ordered array matches how they're consumed.

mnadareski · 2026-05-28T13:20:00Z

+            parameters = Regex.Replace(parameters, $@"(^|\s)--drive=(?:""{escapedDrive}/?""|{escapedDrive}/?)(?=\s|$)", "$1");
+            parameters = Regex.Replace(parameters, $@"(^|\s)--drive\s+(?:""{escapedDrive}/?""|{escapedDrive}/?)(?=\s|$)", "$1");


The drive names need to be escaped for Redumper? I'm having a hard time following what these regex statements are actually doing to the resulting parameters.

On macOS the optical drive mounts under /Volumes/, and Redumper there doesn't take the --drive argument the way it does on Windows. These statements strip any --drive=<vol> / --drive <vol> matching the mounted volume back out of the parameter string. Regex.Escape(driveName) is only there to make the volume name match literally — mounted volume names can contain spaces or regex metacharacters, so without escaping the pattern could misbehave. I can add an inline comment spelling that out.

Added the inline comments spelling this out (commit 548f0ff): one note on Regex.Escape (mounted volume names under /Volumes/ can contain spaces or regex metacharacters, so the name is escaped to match literally), and one on the two Regex.Replace calls (they strip the mounted drive's --drive=<name> / --drive <name> argument — quoted or unquoted, optional trailing slash — because Redumper on macOS doesn't take the drive argument the way it does on Windows).

To be clear on the two parts: the Regex.Escape and what the Regex.Replace calls do are as described above (escaping is only to match the volume name literally in the .NET regex; the replaces strip the mounted-volume --drive argument). The underlying reason the argument is stripped specifically on macOS — i.e. exactly how Redumper expects the drive to be specified there — is best confirmed by @Deterous, who authored this method; my explanation of that part is an inference rather than something I can state authoritatively.

mnadareski · 2026-05-28T13:22:09Z

+        private static NativeMenuItem CreateNativeMenuItem(string header, Action action)
+        {
+            var item = new NativeMenuItem { Header = CleanMenuHeader(header) };
+            item.Click += (_, _) => action();
+            return item;
+        }


In a similar vein to the WireEvents methods, is there a reason that what is being passed in isn't just a Func or Action with the correct parameters for the event handler?

They are delegates — the parameters are named delegate types rather than Func/Action, but functionally the same. The view model takes these callbacks (log output, user-message display, media-info window) so it can drive those UI concerns without depending on the View layer, keeping the MVVM separation. If the preference is to type them as Func<>/Action<> directly, I can switch the signatures.

mnadareski · 2026-05-28T13:24:13Z

+{
+    public partial class MainWindow : WindowBase
+    {
+        private const double ExpandedLogPanelExtraHeight = 40;


I'm a bit surprised that dimensions are allowed to have fractional values in Avalonia.

Avalonia measures layout in device-independent pixels as double throughout (Width/Height/Margin etc. are all double), so fractional values are expected rather than special. The constant here is a whole number — it's just typed as double to match the layout API.

…d handler for event wiring

…-only volume label check

whatev.indus and others added 30 commits April 29, 2026 06:31

Add initial Avalonia UI port

c4ab647

Polish Avalonia UI parity

ae62a8a

Run CI on Avalonia branch and PRs

2820e2d

Grant workflow contents write permission

8b7ce49

Publish Avalonia artifacts in CI

2ad5fc9

Scale down main Avalonia UI

41d387c

Shrink main Avalonia UI by 20 percent

0ce6c7d

Revert "Shrink main Avalonia UI by 20 percent"

d8e52f4

This reverts commit 0ce6c7d.

Refine Avalonia main window layout

9866e7f

Package macOS Avalonia build as app bundle

8a63b2a

Ad-hoc sign macOS app bundle

3ea51c1

Add macOS app bundle icon

34fb8b9

Compact Avalonia submenu layouts

c8fca2f

Tighten Options path and login rows

3b8fe3b

Refine Avalonia window layouts

b96896d

Refine dark mode styling

db00ae4

Add versioned release publishing

2d5ce81

Prepare Avalonia release v0.1.1

93da37a

Prepare Avalonia v0.1.2 release

3064452

Prepare Avalonia v0.1.3 release

dc26ccf

Prepare Avalonia v0.1.4 release

d05ec96

Update release artifact naming

cd17cef

Release 0.1.5 UI refresh

f6a8893

Remove obsolete TODO

0feac87

Release 0.1.6 UI refinements

99b7ac6

Rename build artifacts to MPF.UI prefix

4123fcd

Prepare Avalonia UI 0.1.7

ab585fb

Prepare Avalonia UI 0.1.8

a803112

Cleanup repository for PR, use SabreTools-style build processes

9473e3f

Merge branch 'SabreTools:master' into avalonia

1f79c7f

Deterous marked this pull request as draft May 6, 2026 01:31

Deterous requested a review from mnadareski May 6, 2026 03:58

Deterous added 2 commits May 14, 2026 11:06

More fixes

c8a3a76

Minor tweaks

81fc1af

mnadareski mentioned this pull request May 27, 2026

Complete macOS app for MPF: signed universal .app with auto-signed dumping tools #977

Open

Deterous marked this pull request as ready for review May 28, 2026 02:53

mnadareski reviewed May 28, 2026

View reviewed changes

whatev.indus added 4 commits May 29, 2026 07:06

Add XML doc comments, region tags, and brace-style cleanup

01ca568

Address PR feedback: hoist constants/arrays to class level, use Opene…

536e1ee

…d handler for event wiring

Add explanatory comments for Mac Redumper drive-arg regex and Windows…

548f0ff

…-only volume label check

Make Windows volume-label check explicit with PlatformID.Win32NT

b76b4cb

		if (displayName.Length > 1)
		displayName = displayName.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);

		parameters = Regex.Replace(parameters, $@"(^\|\s)--drive=(?:""{escapedDrive}/?""\|{escapedDrive}/?)(?=\s\|$)", "$1");
		parameters = Regex.Replace(parameters, $@"(^\|\s)--drive\s+(?:""{escapedDrive}/?""\|{escapedDrive}/?)(?=\s\|$)", "$1");

Conversation

Deterous commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mnadareski left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Deterous commented May 6, 2026 •

edited

Loading

mnadareski left a comment •

edited

Loading