refactor branch: MVVM cleanup + slop sweep + CI silence (28 commits)#20
Merged
Conversation
The Must/Should/MustNot construction was duplicated three times with shadowed pattern-match names (visualVm, visualVm2, visualVm3), one cast per branch. Extract three helpers and cast once. - AppendVisualClauses: walks FilterItem source, logs, skips nulls - AppendBannedItemsAsMustNot: unwraps BannedItems wrapper into MustNot - AppendKeyedClauses: ItemConfigs keyed-lookup path Behavior preserved: ConvertFilterItemToClause already returns null for BannedItems operators, so they remain skipped from Must/Should and are surfaced separately into MustNot via the unwrap helper, matching the existing JSON config schema. Method shrinks from 130 lines to 46; total file -15 lines.
…ading
LoadConfigIntoState had three near-identical foreach blocks for
Must/Should/MustNot. Each:
- null-checked the source list
- iterated each clause
- generated a fresh ItemConfigs key
- converted the clause via ConvertClauseToItemConfig
- registered in ItemConfigs and appended the key to the matching
Selected* ObservableCollection
Replace with a single LoadClausesIntoSelection helper invoked three
times. Behavior identical: same null guard, same key generation order,
same conversion call, same target collections.
-14 lines.
…/MotelyJAML The repo declares src/MotelyJAML as a git submodule (.gitmodules), but actions/checkout@v4 only fetches it when given submodules: true|recursive. Without that, dotnet restore runs against an empty src/MotelyJAML/ and exits 1 before any C# is compiled — which is what was happening on PR #20. The fix is the same one already applied to release-windows/linux/macos and deploy-browser.yml. This brings build-browser.yml in line with them. Pre-existing breakage; not introduced by the FiltersModalViewModel refactor.
Same MotelyJAML submodule issue as build-browser.yml — release-browser.yml already had fetch-depth: 0 but not submodules: recursive, so dotnet restore would die on tags. Brings parity with release-windows/linux/macos.
…adTabContent CreateLoadTabContent inlined the same 15-element deck name array twice and the same 8-element lowercase stake name array twice (~75 lines total of copy-paste, four sources of drift across one method). Changes: - Add private static readonly StakeNamesByIndex (already had DeckDisplayValues derived from Enum.GetNames at the class level, so reuse that for decks) - Add private ApplyDeckFromConfig / ApplyStakeFromConfig helpers - Replace all four inline blocks with single-line helper calls - Remove dead-code: GetDeckName(int) and GetStakeName(int) had zero callers in the file (private methods, not virtual, not [JsonInclude]'d, not used by source generators) -81 lines net. File: 2043 -> 1962. Cumulative across 3 file commits: -110 lines from the original 2072.
Per Avalonia's Native AOT guidance, compiled bindings are mandatory (not optional) when reflection is disabled. Desktop already sets IlcDisableReflection=true and Browser uses TrimMode=full, so any binding that falls back to reflection would crash at runtime, not just be slow. The shared BalatroSeedOracle.csproj sets the prop, but heads (Desktop, Browser, Android, iOS) don't inherit MSBuild props from referenced projects — only from Directory.Build.props upward. Hoisting guarantees every head's XAML compiler gets the same default. Pairs with the next commit which removes the now-redundant local prop from BalatroSeedOracle.csproj.
Previous commit (c2b3bc6) accidentally serialized the XML with HTML entity encoding (< / >) instead of literal angle brackets, producing an unparseable MSBuild file. Re-push with raw XML. Same intent as before: hoist AvaloniaUseCompiledBindingsByDefault so the prop applies to all heads, not just the shared library, and pairs with the follow-up that removes the redundant local prop from the shared csproj.
…edBindingsByDefault The prop is now set in Directory.Build.props so every project (heads included) gets it. Keeping the duplicate here is just drift surface.
…src/ Same content as root BROWSER_SEARCH_FIX_PLAN.md but with a hash-prefixed filename, sitting inside the source tree. Pure slop.
Its top items (006-016: replace == null with is null in FiltersModalViewModel) reference patterns already fixed — verified by grep finding zero == null / != null in that file. List drifted away from reality and is misleading to future agents.
…doc) File reads as a "✅ All Fixes Applied" / "Build Status: ✅" past-tense report. Belongs in PR description, not in the repo root.
… code is the source of truth now)
Repo previously had publish_log.txt, publish_retry.txt, publish_retry_2.txt, temp_duckdb_ops.txt, src/BalatroSeedOracle/build_output.txt all committed by accident. Just deleted them in earlier commits — adding ignore patterns to keep them from coming back.
OptimusPi
changed the title
…or catches Two surgical strikes against repeated boilerplate: 1. ToggleWidget helper + 6 expression-bodied [RelayCommand] methods. Each toggle was 8 lines of "if disabled return; play sound; flip flag"; now they're one-line lambda-style commands calling the shared helper. The methods stay separate so CommunityToolkit.Mvvm's source generator still produces the corresponding ICommand properties. 2. HandleModalOpenError helper + 3 modal-launch catch blocks (Editor / Analyze / Tool) collapsed from ~10 lines each to 3 lines. SeedSearch uses the same helper after first clearing ActiveModal (which the other three don't need). Behavior preserved: same DebugLogger key, same user-facing error format, same IsModalVisible reset path.
…ate persistence - Remove unused private GetDatabasePath() (no callers anywhere in the file). - Remove no-op stub LoadSavedProgressAsync() and inline its trivial body (ProgressPercent = 0.0) at the single call site in OnContinueSearchChanged. The method only existed because the saved-progress feature was removed when Motely took over DB ownership; the comment in the method even said so. Net: -22 lines, no behavior change.
User explicitly requested CI go silent. All 8 workflow files preserved so they can be triggered manually via the Actions UI when needed.
…h constructor-injected field IAudioManager was already registered for DI and injected via the constructor as _audioManager. Using ServiceHelper to grab it inside PlayButtonClickSound bypassed the constructor injection that was already wired up. Now: use the existing _audioManager field directly. No DI changes needed.
OptimusPi
changed the title
Owner
Author
🤝 Handoff for next Claude sessionBranch: What's been refactored (28 commits, ~150 lines slop killed)
What's known-deferred
Don't repeat these mistakes
Reusable patterns established
How to verify locallygit fetch && git checkout claude/refactor-legacy-code-zol2f
dotnet build src/BalatroSeedOracle.DesktopIf errors: paste first 30 lines of red, fixes are surgical. User-facing contextUser is Generated by Claude Code |
Code-behind shrunk from 10.6KB / ratio 3.53x to 4.7KB / ratio 0.95x. Animation moved to XAML via <Style.Animations> on Border.modal-border, sizing moved to XAML via class selector keyed off the Squeeze styled property. Back-navigation logic remains in code-behind because BackClicked is part of the public surface consumed by ModalHelper, ToolsModal, DayLatroWidget, AudioVisualizerSettingsModal (subclass) and BalatroMainMenuViewModel; converting to a [RelayCommand] would break all those callsites without delivering MVVM value, since the control is event-driven by design and is reused with arbitrary content. The static ShowModal() anti-pattern was deleted - it had zero callers (orchestration already lives on BalatroMainMenuViewModel and the ModalHelper extension method). SetContent's redundant InvalidateVisual/UpdateLayout debug spam was removed.
…izing in XAML Pairs with the code-behind refactor. Adds two style blocks: 1. Grid.modal-size with default 1080x600 and a UserControl.squeeze override that switches to NaN/NaN/MaxWidth=700/MaxHeight=500. The Squeeze styled property is bound to Classes.squeeze on the root UserControl via RelativeSource Self, replacing the imperative ApplySqueezeSizing() method. 2. Border.modal-border with a Style.Animations one-shot keyframe animation (Opacity 0->1, Margin 0,-24,0,24 -> 0) that runs once on first style match (i.e. on Loaded), replacing the imperative AnimateIn() method and Dispatcher.UIThread.Post tick. This is the Avalonia-idiomatic pattern for intro animations independent of property change.
…Model Slim ResultsTab.axaml.cs from 296 to 76 lines (13.4KB -> 2.9KB) by relocating the Parquet/CSV/DuckDB export pipeline, favorites, analyze-modal, and pop-out window logic into SearchModalViewModel (its existing DataContext). The view now only forwards SortableResultsGrid events to VM methods and handles window construction in response to a VM event (ShowDataGridResultsRequested), matching the established MinimizeToDesktopRequested pattern. - App.GetService<IResultsDatabaseExporter|IParquetExporter|SearchManager> calls removed; services are constructor-injected into SearchModalViewModel via DI (optional, like NotificationService) so the Browser platform stays lean. - new Windows.DataGridResultsWindow(...) stays in the View; VM only resolves the ActiveSearchContext + filter name and raises an event. - Public ForceRefreshResults(...) preserved as a shim for the external caller in SearchModalViewModel.LoadExistingResults (line 1421). - ServiceCollectionExtensions: SearchModalViewModel registration switched to factory form so optional exporter services flow in.
…odel Switch SearchModalViewModel registration to factory form so IResultsDatabaseExporter and IParquetExporter (platform-specific, registered by Desktop/Browser projects) flow into the VM via GetService<>(). Supports the ResultsTab MVVM cleanup that moves export logic out of code-behind.
SearchModalViewModel now exposes ExportSearchResultsAsync, OpenAnalyzeModalForSeed, AddSeedToFavorites, RequestPopOutResults, and a ShowDataGridResultsRequested event so ResultsTab.axaml.cs stays UI-only. IResultsDatabaseExporter and IParquetExporter are optional ctor params (default null) to keep Browser DI lean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch summary
Long refactor branch on
claude/refactor-legacy-code-zol2f. Per direction: no separate PRs, no compile-test (no local toolchain), CI silenced for the duration. Review the diff, pull locally to build.What landed (28 commits)
Wave 1 — FiltersModalViewModel + Slop Sweep
FiltersModalViewModel.cs: 3 dedupe refactors (-110 lines, ~5%) —BuildConfigFromCurrentStatetriplet collapse,LoadConfigIntoStateforeach dedupe, deck/stake constants extracted.gitignoreblockspublish*.txt,build_output.txt,temp_*.txtso build artifacts don't leak againWave 2 — Build/AOT correctness
AvaloniaUseCompiledBindingsByDefault=truefrom one csproj toDirectory.Build.propsso all 5 projects inherit it (mandatory under existingIlcDisableReflection=trueandTrimMode=full)BalatroSeedOracle.csprojbuild-browser.yml,release-browser.yml) — though now moot since CI is silentWave 3 — More ViewModels
BalatroMainMenuViewModel.cs:ToggleWidgethelper consolidates 6× toggle boilerplate;HandleModalOpenErrorhelper consolidates 3× modal-launch catch blocks (~-30 lines net)SearchModalViewModel.cs: deleted deadGetDatabasePath()andLoadSavedProgressAsync()no-op stub (-22 lines)BalatroMainMenuViewModel.PlayButtonClickSound: stopped usingServiceHelper.GetService<IAudioManager>()when_audioManageris already a constructor-injected field on the same class (-2 lines)Wave 4 — CI silence
build-browser.yml,deploy-browser.yml,publish-motely-wasm.yml,release.yml) — all converted toworkflow_dispatch:onlyworkflow_call/workflow_dispatch)deploy-browser.ymlandpublish-motely-wasm.ymllost theirworkflow_dispatch.inputs(env / version). Their referencing steps will no-op on manual runs. Restore inputs when you actually need to deploy.What's NOT touched (with reason)
BalatroMainMenu.axaml.csgod-class (85KB code-behind, ratio 4.20) — biggest single MVVM violation in the repo. Decomposition is multi-day work; not attempted in this branch. TheBalatroMainMenuViewModelis already set up to absorb the logic (it has[ObservableProperty],[RelayCommand], modal events) — seei-trust-you-rosy-snail.mdplan file for the phased approach.FilterListViewModel,FiltersModalViewModel,FilterSelectionModalViewModel— each has 3-8ServiceHelper.GetServicecalls. The audit underestimated. Fixing them requires DI registration extension + manual-instantiation callsite updates across multiple files. Non-trivial; skipped to avoid breaking unverified callsites.AudioVisualizerSettingsWidgetViewModel.cs(71KB) — earlier scan flagged 17 "dead" private methods; verification found all 17 are actually live (15 are[RelayCommand]-bound or callBalatroMainMenupublic methods, 1 has internal callers). Zero deletions safe. File left as-is.MessageBox/IInteractionServicepattern — 6-7 ViewModels still callMessageBoxManager/ShowDialogdirectly. New service interface needs designing for Browser/Desktop split. Deferred.x:DataTypesweep on.axaml— with compiled bindings hoisted, every view needsx:DataTypedeclared. Sample (MainWindow.axaml) had it. Full audit not run.Stats
How to verify locally
If it errors: paste the first 30 lines of red output, fixes are surgical.
Plan for next session
i-trust-you-rosy-snail.md(in plan file location) outlines 5 phases for finishing MVVM. Phase 0 (CI silence) and parts of Phases 1, 2 are done. Remaining: Phase 2 (BalatroMainMenu decomposition), Phase 3 (smaller code-behind cleanups), Phase 4 (IInteractionService), Phase 5 (x:DataTypeaudit).