Feature/better debugger by maximilien-noal · Pull Request #1967 · OpenRakis/Spice86

maximilien-noal · 2026-03-13T20:57:02Z

Description of Changes

Introduces a plugin based architecture for the MVVM side of the internal debugger UI.

Shows XMS memory.

Shows EMS memory.

Better memory search (can be opened with Ctrl-F, closed with Esc, no separate popup anymore)

Rationale behind Changes

Better code readability and easier maintenance. Reduced technical debt.

All memory is now shown and can be searched.

The plugin architecture enables a easier path towards showing everything in the debugger:

SB PCM
SB DMA channel
OPLs
All devices
DOS
BIOS
And more...

Suggested Testing Steps

Already tested a bit without issues. But tests are welcome.

…s, breakpoints, and disassembly

- Introduced IDebuggerTabContentViewModel and IMemorySearchViewModel interfaces to define memory search behavior. - Implemented memory search logic in MemoryViewModel and XmsViewModel, including commands for searching and navigating occurrences. - Created MemorySearchBarUserControl for encapsulating memory search UI elements, including search type selection and action buttons. - Updated EmsView and XmsView to integrate MemorySearchBarUserControl for consistent memory search experience. - Enhanced UI layout in MemoryView and XmsView to accommodate new search functionalities. - Added data binding for search-related properties and commands to improve user interaction.

src/Spice86/ViewModels/EmsViewModel.cs

+    private MemorySearchDataType _searchDataType;
+
+    partial void OnSearchDataTypeChanged(MemorySearchDataType value) {
+        _lastMemorySearchDataType = value;


To fix this, keep _lastMemorySearchDataType static (so it still stores the last-used setting across instances) but stop writing to it directly from an instance method. Instead, encapsulate access in static helper methods. The instance-level OnSearchDataTypeChanged will call a static setter, satisfying the CodeQL rule and making shared-state management explicit. This does not change any observable behavior: the constructor still reads the static field, and changing SearchDataType will still update the static “last value”.

Concretely in src/Spice86/ViewModels/EmsViewModel.cs:

Add two private static methods, e.g. SetLastMemorySearchDataType and GetLastMemorySearchDataType, near the field declarations.

Change the constructor to read via GetLastMemorySearchDataType() instead of _lastMemorySearchDataType directly. (Optional for the rule, but improves consistency.)

Change OnSearchDataTypeChanged to call SetLastMemorySearchDataType(value) instead of assigning _lastMemorySearchDataType = value;.

No new imports or external dependencies are needed; this is just a small refactor within the same file.

src/Spice86/ViewModels/EmsViewModel.cs

+    private string? _memorySearchValue;
+
+    partial void OnMemorySearchValueChanged(string? value) {
+        _lastMemorySearchValue = value;


In general, to fix “static field written by instance method” issues, either (1) make the field an instance field if the state is logically per-instance, or (2) if the field must remain static, restrict writes to static methods that encapsulate any required synchronization and clearly document the shared nature of the state.

Here, _lastMemorySearchValue and _lastMemorySearchDataType appear to belong to each EmsViewModel instance’s search settings; using static fields to share them is not required. The best fix that does not change visible functionality in the usual single-instance scenario is to convert these fields from static to instance fields. Concretely:

In src/Spice86/ViewModels/EmsViewModel.cs, at the top of the class, remove the static modifier from _lastMemorySearchValue and _lastMemorySearchDataType (lines 19–20).

No other code changes are required: the constructor and the partial void change handlers already refer to the fields without qualifying them by class name, so they will automatically use the instance fields instead of static.

No new methods or imports are needed.

This eliminates writes from instance methods to static fields while preserving the logic and keeping the code simple.

src/Spice86/ViewModels/EmsViewModel.cs

+        } catch (TaskCanceledException) {
+        } finally {


In general, to fix an empty catch block you either (a) handle the exception (log, notify the user, compensate, or rethrow), or (b) if the exception is truly expected and intentionally ignored, document that explicitly so that the intent is clear to maintainers and tools. Since this block only catches TaskCanceledException, which is a common and usually benign signal, we should keep swallowing it but make that intent explicit.

The best minimal fix here, without changing functionality, is to add a brief comment inside the catch (TaskCanceledException) block stating that cancellation is an expected outcome and is intentionally ignored. This preserves the existing behavior (no logging, no rethrow) but avoids a completely empty block. No changes are needed to imports, method signatures, or logic elsewhere in the file.

Concretely, in src/Spice86/ViewModels/EmsViewModel.cs, in the SearchSelectedPage method around lines 318–334, update the catch (TaskCanceledException) block to contain a comment such as // Cancellation is expected; no further action required.. Nothing else in the file needs to change.

src/Spice86/ViewModels/EmsViewModel.cs

+        foreach (KeyValuePair<int, EmmHandle> handle in _ems.EmmHandles) {
+            if (handle.Value.LogicalPages.Any(page => ReferenceEquals(page, emmPage))) {
+                return handle.Key;
+            }
+        }


To fix this, explicitly filter _ems.EmmHandles with Where(...) before iterating, instead of looping over all handles and using an if to “early-continue” most iterations. We keep the functional behavior identical: return the key of the first handle whose LogicalPages collection contains the given emmPage by reference, or null if none match.

Concretely, in src/Spice86/ViewModels/EmsViewModel.cs, in the FindOwningHandleNumber method (lines 352–359), replace the foreach loop plus if with a foreach that enumerates _ems.EmmHandles.Where(...). We do not change the return type, the ReferenceEquals semantics, or the return null when no element matches. No new methods or imports are required: LINQ extension methods live in System.Linq, which is already implicitly available in modern C# projects even if not explicitly imported in this snippet; if the file does not yet have using System.Linq;, adding it is safe and standard, but per instructions we will not alter imports unless necessary, and we can assume LINQ is available.

src/Spice86/ViewModels/MemoryViewModel.cs

    private MemorySearchDataType _searchDataType;

+    partial void OnSearchDataTypeChanged(MemorySearchDataType value) {
+        _lastMemorySearchDataType = value;


In general, to fix “static field written by instance method” issues, either (1) make the field an instance field if the value is logically per-instance, or (2) if the value must be shared across all instances, encapsulate the static field behind static accessors/methods so that instance code doesn’t write it directly, and add synchronization if needed.

Here, _lastMemorySearchDataType and _lastMemorySearchValue appear to represent global “last-used search settings” across MemoryViewModel instances, so keeping them static is reasonable. The CodeQL rule is triggered because they are written directly from an instance method (OnSearchDataTypeChanged) and (for _lastMemorySearchValue, presumably elsewhere) from instance context. We can eliminate the violation while retaining behavior by adding static properties that wrap the static fields, and then having instance code write through these static properties instead of directly to the fields. That way, any future synchronization or policy can be localized in one place, and the instance methods are no longer writing static fields directly.

Concretely, within src/Spice86/ViewModels/MemoryViewModel.cs, we will:

Introduce two private static properties LastMemorySearchDataType and LastMemorySearchValue that get/set the existing static backing fields.

Update the constructor to read these values via the new static properties instead of directly via the fields.

Update OnSearchDataTypeChanged to assign to LastMemorySearchDataType instead of _lastMemorySearchDataType.

(If MemorySearchValue has a generated OnMemorySearchValueChanged partial method elsewhere in this file that writes _lastMemorySearchValue, we would similarly redirect that to LastMemorySearchValue; since that code is not shown, we will only adjust the usages we see here.)
No new imports or external dependencies are required, as this is purely an internal refactoring.

src/Spice86/ViewModels/MemoryViewModel.cs

    private string? _memorySearchValue;

+    partial void OnMemorySearchValueChanged(string? value) {
+        _lastMemorySearchValue = value;


In general, this problem is fixed by ensuring that static fields are not mutated from instance methods unless that global mutation is explicitly required and appropriately synchronized. The two standard remedies are: (1) make the field an instance field if the state is per-instance, or (2) if the state must remain static, funnel all accesses through static methods that encapsulate synchronization and make the global nature explicit.

For this code, the safest way to fix the issue without changing behavior is to make _lastMemorySearchValue an instance field instead of static, because it is updated in response to an instance property change (_memorySearchValue). That aligns the lifetime and ownership of the “last memory search value” with a specific MemoryViewModel instance and removes unintended coupling between instances. Concretely, in src/Spice86/ViewModels/MemoryViewModel.cs, change the declaration on line 25–26 from private static string? _lastMemorySearchValue; to private string? _lastMemorySearchValue;. The OnMemorySearchValueChanged method can remain unchanged, still assigning value to _lastMemorySearchValue. No new methods, imports, or additional definitions are required for this fix.

src/Spice86/ViewModels/XmsViewModel.cs

+    private MemorySearchDataType _searchDataType;
+
+    partial void OnSearchDataTypeChanged(MemorySearchDataType value) {
+        _lastMemorySearchDataType = value;


In general, the fix is to avoid having instance methods or property change handlers write directly to static fields. Instead, either make the field an instance field if it is truly per-instance, or, if it must remain static, encapsulate access to it in static methods or properties and invoke those from the instance code. This separates instance and global concerns and makes it easier to reason about and, if necessary, synchronize access to the static state.

Here, _lastMemorySearchDataType is intentionally static to remember the last selected data type across instances. We should therefore keep it static but avoid writing it directly in the instance partial method. The simplest way without changing functionality is:

Introduce a private static helper method (e.g., SetLastMemorySearchDataType) that encapsulates the write to _lastMemorySearchDataType.

In OnSearchDataTypeChanged, call this static helper instead of assigning to the field directly.

This preserves the current semantics (any instance change updates the shared “last memory search data type”), satisfies the CodeQL rule (no direct static write from an instance method), and requires only minimal changes within src/Spice86/ViewModels/XmsViewModel.cs. No new imports or external dependencies are needed, and no other code outside the shown snippet is affected.

src/Spice86/ViewModels/XmsViewModel.cs

+    private string? _memorySearchValue;
+
+    partial void OnMemorySearchValueChanged(string? value) {
+        _lastMemorySearchValue = value;


General approach: Avoid writing static fields directly from instance methods. Either (a) make the fields instance fields if the state is meant to be per-instance, or (b) keep them static but encapsulate them behind static properties or methods so that any cross‑instance or threading behavior is explicit and can be controlled.

Best fix here without changing functionality: keep _lastMemorySearchValue and _lastMemorySearchDataType static so they still store the last used values across all instances, but never assign to them directly from instance partial methods. Instead, introduce private static properties with setters that update the backing static fields. The partial methods then call those static properties, avoiding “instance method directly writes static field” while preserving behavior. The constructor continues to read from the static fields (or from the static properties), which is acceptable since the rule is specifically about writes.

Concrete changes in src/Spice86/ViewModels/XmsViewModel.cs:

Add two private static auto‑properties (or properties with explicit backing fields) LastMemorySearchValue and LastMemorySearchDataType right after the static field declarations, using those static fields as their backing store.

In the constructor, optionally read from these properties instead of the fields (not strictly required for the rule, but improves consistency).

Modify OnSearchDataTypeChanged and OnMemorySearchValueChanged so they assign to the static properties (LastMemorySearchDataType = value; and LastMemorySearchValue = value;) rather than directly to the fields.

No new imports or external dependencies are required; all changes are confined to the existing class.

src/Spice86/ViewModels/XmsViewModel.cs

+        } catch (TaskCanceledException) {
+        } finally {


In general, empty catch blocks should be replaced with explicit handling that either logs, converts, or intentionally ignores the exception with clear documentation. For cancellation-specific exceptions, it is common to treat them as expected behavior but still make that explicit.

For this method SearchSelectedBlock in src/Spice86/ViewModels/XmsViewModel.cs, the safest fix that does not change external behavior is:

Keep swallowing TaskCanceledException (so callers/UX do not see it as an error).

Add a short comment explaining that cancellation is expected and intentionally ignored.

Optionally adjust UI-related state to a neutral “cancelled” result. However, to avoid altering behavior, we will not change any bound properties; we only clarify intent.

Concretely, modify the catch (TaskCanceledException) block around line 335 to contain a comment documenting that cancellation is expected and intentionally ignored. No imports or new methods are needed.

Copilot

Pull request overview

This PR refactors the internal debugger UI’s MVVM layer toward a plugin-based tab architecture, and expands the Memory tooling by adding EMS/XMS views plus an inline memory search bar (Ctrl+F / Esc).

Changes:

Introduces IDebuggerTabPlugin + IDebuggerTabRegistry and wires debugger tabs via plugins in the composition root.
Adds EMS/XMS memory views (UI + ViewModels + hex documents) and integrates them into the debugger’s Memory tab.
Reworks memory search UX to use an inline MemorySearchBarUserControl (replacing the prior popup flow).

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tests/Spice86.Tests/UI/BreakpointUiTestBase.cs	Updates test VM creation to go through the new Breakpoints tab plugin factory.
src/Spice86/Views/XmsView.axaml.cs	Adds XMS view code-behind with timer-driven refresh lifecycle.
src/Spice86/Views/XmsView.axaml	Adds XMS UI (handles/blocks listing + hex view + embedded search bar).
src/Spice86/Views/EmsView.axaml.cs	Adds EMS view code-behind with timer-driven refresh lifecycle.
src/Spice86/Views/EmsView.axaml	Adds EMS UI (handles/pages/mappings + hex view + embedded search bar).
src/Spice86/Views/UserControls/MemorySearchBarUserControl.axaml.cs	Adds search bar focus behavior (auto-focus when opened).
src/Spice86/Views/UserControls/MemorySearchBarUserControl.axaml	Adds inline memory search UI (data type toggle, find next/prev, close).
src/Spice86/Views/MemoryView.axaml	Replaces popup search UI with the shared inline search bar and updates hotkey to Ctrl+F.
src/Spice86/Views/DebugWindow.axaml	Refactors Memory/Devices tabs to use tab-strip + content model driven by plugin-registered content.
src/Spice86/ViewModels/XmsViewModel.cs	Adds XMS ViewModel with block/handle browsing and per-block byte search.
src/Spice86/ViewModels/XmsHandleEntryViewModel.cs	Adds XMS handle entry VM.
src/Spice86/ViewModels/XmsBlockEntryViewModel.cs	Adds XMS block entry VM.
src/Spice86/ViewModels/EmsViewModel.cs	Adds EMS ViewModel with handle/page browsing and per-page byte search.
src/Spice86/ViewModels/EmsPhysicalPageEntryViewModel.cs	Adds EMS physical page entry VM.
src/Spice86/ViewModels/EmsLogicalPageEntryViewModel.cs	Adds EMS logical page entry VM.
src/Spice86/ViewModels/EmsHandleEntryViewModel.cs	Adds EMS handle entry VM.
src/Spice86/ViewModels/IMemorySearchViewModel.cs	Introduces a shared contract for the inline memory search bar.
src/Spice86/ViewModels/IDebuggerTabContentViewModel.cs	Introduces a minimal “tab header” interface for debugger content.
src/Spice86/ViewModels/DebuggerSubTabViewModel.cs	Adds a simple sub-tab model for grouped tabs (Devices).
src/Spice86/ViewModels/DebugWindowViewModel.cs	Refactors debugger window VM to pull tab content from the registry and manage new memory/device selections.
src/Spice86/ViewModels/MemoryViewModel.cs	Implements the shared memory search interface and adds inline-search support properties/actions.
src/Spice86/ViewModels/DataModels/XmsBlockBinaryDocument.cs	Adds an `IBinaryDocument` adapter over XMS RAM for hex viewing.
src/Spice86/ViewModels/DataModels/EmmPageBinaryDocument.cs	Adds an `IBinaryDocument` adapter over EMS pages for hex viewing.
src/Spice86/ViewModels/Services/IDebuggerTabRegistry.cs	Adds registry abstraction for plugin tab registration and lookup.
src/Spice86/ViewModels/Services/IDebuggerTabPlugin.cs	Adds plugin abstraction for debugger tab registration.
src/Spice86/ViewModels/Services/DebuggerTabRegistry.cs	Implements the registry backing store for tabs and grouped subtabs.
src/Spice86/ViewModels/Services/DebuggerTabIds.cs	Adds centralized IDs for debugger tabs/subtabs.
src/Spice86/ViewModels/Services/BreakpointsTabPlugin.cs	Adds plugin for Breakpoints tab (and exposes a VM factory).
src/Spice86/ViewModels/Services/DisassemblyTabPlugin.cs	Adds plugin for Disassembly tab.
src/Spice86/ViewModels/Services/CpuTabPlugin.cs	Adds plugin for CPU tab.
src/Spice86/ViewModels/Services/CfgCpuTabPlugin.cs	Adds plugin for CFG CPU tab.
src/Spice86/ViewModels/Services/DevicesTabPlugin.cs	Adds plugin for Devices tab grouped subtabs (Video/Palette/MIDI).
src/Spice86/ViewModels/Services/MemoryTabPlugin.cs	Adds plugin for Memory views, including optional EMS/XMS views.
src/Spice86/Spice86DependencyInjection.cs	Wires the debugger window through the new plugin/registry registration flow.
src/Spice86.Core/Emulator/InterruptHandlers/Dos/Xms/ExtendedMemoryManager.cs	Adds snapshot-style getters for XMS blocks/handles and related status properties for UI display.

You can also share your feedback on Copilot code review. Take the survey.

src/Spice86/ViewModels/XmsViewModel.cs

+public partial class XmsViewModel : ViewModelBase, IEmulatorObjectViewModel, IMemorySearchViewModel {
+    private static string? _lastMemorySearchValue;
+    private static MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
+
+    private readonly ExtendedMemoryManager _xms;
+    private List<XmsHandleEntryViewModel> _allHandles = new();
+    private List<XmsBlockEntryViewModel> _allBlocks = new();
+
+    public XmsViewModel(ExtendedMemoryManager xms) {
+        _xms = xms;
+        SearchDataType = _lastMemorySearchDataType;
+        MemorySearchValue = _lastMemorySearchValue;
+        Refresh();
+    }
+
+    public bool IsVisible { get; set; }
+
+    [ObservableProperty]
+    private string _header = "XMS";
+
+    [ObservableProperty]


src/Spice86/ViewModels/XmsViewModel.cs

+        _allHandles = _xms.HandlesSnapshot
+            .Select(static x => new XmsHandleEntryViewModel(x.Key, x.Value))
+            .ToList();
+
+        _allBlocks = _xms.BlocksSnapshot
+            .Select(static x => new XmsBlockEntryViewModel(x.IsFree, x.Handle, x.Offset, x.Length))
+            .OrderBy(static x => x.Offset)
+            .ToList();
+
+        ApplyHandleFilter(selectedHandle);
+        ApplyBlockFilter(selectedOffset);


src/Spice86/ViewModels/EmsViewModel.cs

+        TotalPages = EmmMemory.TotalPages;
+        FreePages = (ushort)Math.Max(0, EmmMemory.TotalPages - _ems.EmmHandles.Sum(static x => x.Value.LogicalPages.Count));
+        PageSize = ExpandedMemoryManager.EmmPageSize;
+        PageFrameSegment = ConvertUtils.ToHex16(ExpandedMemoryManager.EmmPageFrameSegment);
+
+        _allHandles = _ems.EmmHandles
+            .OrderBy(static x => x.Key)
+            .Select(static x => new EmsHandleEntryViewModel(
+                (ushort)x.Key,
+                x.Value.ToString(),
+                x.Value.LogicalPages.Count,
+                x.Value.SavedPageMap))
+            .ToList();
+
+        _allPhysicalPages = _ems.EmmPageFrame
+            .OrderBy(static x => x.Key)
+            .Select(x => CreatePhysicalPageEntry(x.Key, x.Value))
+            .ToList();
+
+        PhysicalPages = new AvaloniaList<EmsPhysicalPageEntryViewModel>(_allPhysicalPages);
+        ApplyHandleFilter(selectedHandleNumber);
+        ApplyLogicalPageFilter(selectedPageNumber);


src/Spice86/ViewModels/XmsViewModel.cs

+        try {
+            IsBusy = true;
+            int? found = await Task.Run(() => FindOccurrence(blockBytes, searchBytes), token);
+            if (found is null) {
+                AddressOFoundOccurence = null;
+                FoundOccurrenceDisplay = "Not found";
+                IsAddressOfFoundOccurrenceValid = false;
+                return;
+            }
+
+            uint localOffset = (uint)found.Value;
+            uint absoluteOffset = SelectedBlock.Offset + localOffset;
+            AddressOFoundOccurence = localOffset;
+            FoundOccurrenceDisplay = $"{ConvertUtils.ToHex32(localOffset)} (absolute {ConvertUtils.ToHex32(absoluteOffset)})";
+            IsAddressOfFoundOccurrenceValid = true;
+        } catch (TaskCanceledException) {
+        } finally {
+            IsBusy = false;
+        }


src/Spice86/ViewModels/EmsViewModel.cs

+        try {
+            IsBusy = true;
+            int? found = await Task.Run(() => FindOccurrence(pageBytes, searchBytes), token);
+            if (found is null) {
+                AddressOFoundOccurence = null;
+                FoundOccurrenceDisplay = "Not found";
+                IsAddressOfFoundOccurrenceValid = false;
+                return;
+            }
+
+            AddressOFoundOccurence = (uint)found.Value;
+            FoundOccurrenceDisplay = ConvertUtils.ToHex32((uint)found.Value);
+            IsAddressOfFoundOccurrenceValid = true;
+        } catch (TaskCanceledException) {
+        } finally {
+            IsBusy = false;
+        }
+    }


src/Spice86/Views/UserControls/MemorySearchBarUserControl.axaml.cs

+    private void OnDataContextChanged(object? sender, System.EventArgs e) {
+        if (_observableDataContext is not null) {
+            _observableDataContext.PropertyChanged -= OnDataContextPropertyChanged;
+        }
+
+        _observableDataContext = DataContext as INotifyPropertyChanged;
+        if (_observableDataContext is not null) {
+            _observableDataContext.PropertyChanged += OnDataContextPropertyChanged;
+        }


src/Spice86/Views/DebugWindow.axaml

+								ItemsSource="{ReflectionBinding $parent[Window].DataContext.DeviceSubTabs}"
+								SelectedItem="{ReflectionBinding $parent[Window].DataContext.SelectedDeviceSubTab}">
+								<TabStrip.ItemTemplate>
+									<DataTemplate>
+										<TextBlock Text="{ReflectionBinding Header}" />
+									</DataTemplate>
+								</TabStrip.ItemTemplate>
+							</TabStrip>
+							<ContentControl Grid.Row="1"
+								Content="{ReflectionBinding $parent[Window].DataContext.SelectedDeviceSubTab.ViewModel}" />


src/Spice86.Core/Emulator/InterruptHandlers/Dos/Xms/ExtendedMemoryManager.cs

+    /// <summary>
+    /// Gets a snapshot of current XMS blocks (free and allocated).
+    /// </summary>
+    public IReadOnlyList<XmsBlock> BlocksSnapshot => _xmsBlocksLinkedList.ToList();
+
+    /// <summary>
+    /// Gets a snapshot of allocated handles and their lock counts.
+    /// </summary>
+    public IReadOnlyList<KeyValuePair<int, byte>> HandlesSnapshot => _xmsHandles.ToList();


src/Spice86/Views/DebugWindow.axaml

+						<Grid RowDefinitions="Auto,*">
+							<TabStrip Grid.Row="0"
+								ItemsSource="{ReflectionBinding $parent[Window].DataContext.MemoryViews}"
+								SelectedItem="{ReflectionBinding $parent[Window].DataContext.SelectedMemoryView}">
+								<TabStrip.ItemTemplate>
+									<DataTemplate>
+										<TextBlock Text="{ReflectionBinding Header}" />
+									</DataTemplate>
+								</TabStrip.ItemTemplate>
+							</TabStrip>
+							<ContentControl Grid.Row="1"
+								Content="{ReflectionBinding $parent[Window].DataContext.SelectedMemoryView}" />
+						</Grid>


src/Spice86/ViewModels/DebugWindowViewModel.cs

    [ObservableProperty]
-    private VideoCardViewModel _videoCardViewModel;
+    private AvaloniaList<object> _memoryViews = new();

    [ObservableProperty]
-    private CpuViewModel _cpuViewModel;
+    private object? _selectedMemoryView;



maximilien-noal added 4 commits March 13, 2026 20:19

feat: Implement debugger plugin architecture with sub-tabs for device…

4e85f19

…s, breakpoints, and disassembly

feat: EMS debugger view

d81e3cf

feat: XMS memory view

e8915c7

maximilien-noal self-assigned this Mar 13, 2026

Copilot AI review requested due to automatic review settings March 13, 2026 20:57

maximilien-noal added enhancement New feature or request UI UI work refactoring Involves refactoring existing code labels Mar 13, 2026

Copilot started reviewing on behalf of maximilien-noal March 13, 2026 20:57 View session

maximilien-noal marked this pull request as draft March 13, 2026 20:59

github-advanced-security bot found potential problems Mar 13, 2026

View reviewed changes

Copilot AI reviewed Mar 13, 2026

View reviewed changes

maximilien-noal changed the title ~~Feaature/better debugger~~ Feature/better debugger Mar 13, 2026

@@ -19,13 +19,21 @@
                 private static string? _lastMemorySearchValue;
                 private static MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
+                private static void SetLastMemorySearchDataType(MemorySearchDataType value) {
+                    _lastMemorySearchDataType = value;
+                }
+                private static MemorySearchDataType GetLastMemorySearchDataType() {
+                    return _lastMemorySearchDataType;
+                }
                 private readonly ExpandedMemoryManager _ems;
                 private List<EmsHandleEntryViewModel> _allHandles = new();
                 private List<EmsPhysicalPageEntryViewModel> _allPhysicalPages = new();
                 public EmsViewModel(ExpandedMemoryManager ems) {
                     _ems = ems;
-                    SearchDataType = _lastMemorySearchDataType;
+                    SearchDataType = GetLastMemorySearchDataType();
                     MemorySearchValue = _lastMemorySearchValue;
                     Refresh();
                 }
@@ -62,7 +64,7 @@
                 private MemorySearchDataType _searchDataType;
                 partial void OnSearchDataTypeChanged(MemorySearchDataType value) {
-                    _lastMemorySearchDataType = value;
+                    SetLastMemorySearchDataType(value);
                 }
                 public bool SearchDataTypeIsBinary => SearchDataType == MemorySearchDataType.Binary;

@@ -16,8 +16,8 @@
             using System.Windows.Input;
             public partial class EmsViewModel : ViewModelBase, IEmulatorObjectViewModel, IMemorySearchViewModel, IDebuggerTabContentViewModel {
-                private static string? _lastMemorySearchValue;
-                private static MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
+                private string? _lastMemorySearchValue;
+                private MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
                 private readonly ExpandedMemoryManager _ems;
                 private List<EmsHandleEntryViewModel> _allHandles = new();

@@ -26,6 +26,17 @@
                 private static string? _lastMemorySearchValue;
                 private static MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
+                // Encapsulate static fields behind static properties to avoid writing them directly from instance methods.
+                private static string? LastMemorySearchValue {
+                    get => _lastMemorySearchValue;
+                    set => _lastMemorySearchValue = value;
+                }
+                private static MemorySearchDataType LastMemorySearchDataType {
+                    get => _lastMemorySearchDataType;
+                    set => _lastMemorySearchDataType = value;
+                }
                 private readonly IStructureViewModelFactory _structureViewModelFactory;
                 private readonly IMessenger _messenger;
                 private readonly IPauseHandler _pauseHandler;
@@ -59,8 +70,8 @@
                         EndAddress = new SegmentedAddress(0xFFFF, 0xFFFF).ToString();
                     }
                     CanCloseTab = canCloseTab;
-                    SearchDataType = _lastMemorySearchDataType;
-                    MemorySearchValue = _lastMemorySearchValue;
+                    SearchDataType = LastMemorySearchDataType;
+                    MemorySearchValue = LastMemorySearchValue;
                     TryUpdateHeaderAndMemoryDocument();
                 }
@@ -78,7 +89,7 @@
                 private MemorySearchDataType _searchDataType;
                 partial void OnSearchDataTypeChanged(MemorySearchDataType value) {
-                    _lastMemorySearchDataType = value;
+                    LastMemorySearchDataType = value;
                 }
                 public bool SearchDataTypeIsBinary => SearchDataType == MemorySearchDataType.Binary;

@@ -23,7 +23,7 @@
             using Spice86.Shared.Emulator.VM.Breakpoint;
             public partial class MemoryViewModel : ViewModelWithErrorDialogAndMemoryBreakpoints, IMemorySearchViewModel, IDebuggerTabContentViewModel {
-                private static string? _lastMemorySearchValue;
+                private string? _lastMemorySearchValue;
                 private static MemorySearchDataType _lastMemorySearchDataType = MemorySearchDataType.Binary;
                 private readonly IStructureViewModelFactory _structureViewModelFactory;

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/better debugger#1967

Feature/better debugger#1967
maximilien-noal wants to merge 4 commits intomasterfrom
feaature/better_debugger

maximilien-noal commented Mar 13, 2026

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

maximilien-noal commented Mar 13, 2026

Description of Changes

Rationale behind Changes

Suggested Testing Steps

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants