Skip to content

Aspid.MVVM 1.1.0#25

Merged
VPDPersonal merged 532 commits into
mainfrom
Version/Aspid.MVVM-1.1.0
Jun 7, 2026
Merged

Aspid.MVVM 1.1.0#25
VPDPersonal merged 532 commits into
mainfrom
Version/Aspid.MVVM-1.1.0

Conversation

@VPDPersonal

@VPDPersonal VPDPersonal commented Oct 13, 2025

Copy link
Copy Markdown
Owner

Summary

Aspid.MVVM 1.1.0 is a large release: the inspector stack is rewritten on top of UI Toolkit, Bindable Properties and ValueViewModel are introduced, GeneralView is merged into MonoView, new binder families ship (AudioSource / LayoutGroup / Dropdown / Selectable / Object Name / OneWayToSource), ViewModelDebugPanel is rebuilt on UI Toolkit, an Aspid.MVVM Settings window prototype lands, virtual binder fields and HeaderGroup attributes are added, the Unity project is relocated into Aspid.MVVM/, the UPM package is renamed com.aspid.mvvmtech.aspid.mvvm and promoted to an embedded local package under Packages/tech.aspid.mvvm, the generators and the analyzer are extracted into submodules (Aspid.Collections ships as a UPM git package, Aspid.FastTools is embedded under Packages/tech.aspid.fasttools), the minimum supported Unity is raised to 6000.0, and a release workflow publishing stable (upm) and preview (upm-preview) UPM subtrees is added (the README carries matching Stable / Preview badges and a preview install section).

This PR is published to the preview channel first: package.json is 1.1.0-beta.1 (upm-preview). The stable 1.0.x line is unaffected.

Full change list and migration rules — see CHANGELOG.md (section [1.1.0-beta.1]) and MIGRATION.md.

Highlights

  • MonoBinder / MonoView / MonoViewModel editor inspectors rewritten on UI Toolkit / VisualElement.
  • ViewModelDebugPanel on UI Toolkit: tabs, persistent search, RelayCommand, bindable / auto-properties.
  • Aspid.MVVM Settings window prototype (AspidToggle, shared USS styling).
  • Source generator: Bindable Properties support, NotifyCanExecuteChangedAll(), keyword fields for set-methods, virtual MonoBinder[] slots auto-emitted for IView<TViewModel> members (opt out via [View(AutoBinderFields = false)]).
  • HeaderGroupAttribute / HeaderGroupStartAttribute / HeaderGroupEndAttribute — named, collapsible inspector foldouts for binders and VM members.
  • GeneralView merged into MonoView; MonoView is now non-abstract, with its own serialized binders list and [RequireBinder] validation.
  • New binders: AudioSource, LayoutGroup, Dropdown, Selectable, Object Name, OneWayToSourceComponentBinders, AnyReverseBinder with nullable support, extended InputField set.
  • Extended binder pipeline: OnReplace / OnMove, batch Replace unrolled into per-item OnReplace, BindSafely / UnbindSafely enriched with owner / memberName / bindable Id.
  • Aspid.FastTools integrated; many editor visuals migrated to FastTools equivalents.
  • The generators and the analyzer are extracted into submodules: Aspid.MVVM.Generators, Aspid.MVVM.Analyzers, Aspid.MVVM.Unity.Generators. Aspid.Collections (tech.aspid.collections) is consumed as a UPM git package; Aspid.FastTools is embedded as a local UPM package (tech.aspid.fasttools).
  • Unity project moved from the repository root into Aspid.MVVM/; the UPM package is renamed com.aspid.mvvmtech.aspid.mvvm, promoted to an embedded local package under Packages/tech.aspid.mvvm, and ships its own package.json with unity = 6000.0 and version = 1.1.0-beta.1.
  • GitHub Actions release workflow that publishes the package to upm (stable) and upm-preview branches with immutable upm/<version> tags, generator-DLL drift verification and CHANGELOG-driven release notes. The README carries matching Stable / Preview version badges and a preview install (#upm-preview) section.
  • Mass XML documentation pass across binders, converters and StarterKit; XmlDocConventions.md added.

Breaking changes

  • The UPM package id is renamed com.aspid.mvvmtech.aspid.mvvm. Consumers must update the dependency key in their Packages/manifest.json (the old id will no longer resolve).
  • The minimum supported Unity is raised to 6000.0.
  • MonoView is no longer abstract — it is a concrete component with its own serialized binders list and [RequireBinder] validation.
  • MonoView.Dispose() no longer destroys the host GameObject — it only calls Deinitialize(). Destroy the GameObject manually via Object.Destroy(gameObject) when needed.
  • MonoBinder.Bind() no longer throws when called on an already-bound binder — it writes a Debug.LogError and returns instead.
  • [AddComponentMenu] paths reorganized (e.g. Collections/Observable List Binder - ViewModelCollection/Observable List Binder – ViewModel).
  • Removed AddComponentContextMenuAttribute, AddPropertyContextMenuAttribute; replaced by AddBinderContextMenuAttribute / AddBinderContextMenuByTypeAttribute with a new signature (Path = "...").
  • StarterKit classes renamed (.meta GUIDs preserved, so prefabs and scenes keep working — game code referencing the old names will not compile):
    • ViewModelObservableListMonoBinderObservableListViewModelMonoBinder
    • ViewModelObservableListBinderObservableListViewModelBinder
    • ViewModelObservableDictionaryBinderObservableDictionaryViewModelBinder
    • ViewModelCollectionMonoBinderCollectionViewModelMonoBinder
    • EnumMonoBinderEditorBaseEnumMonoBinderEditor

Linked submodule PRs

Test plan

  • Open the project in Unity 6000.0 LTS (the new minimum) — compiles cleanly, Edit / Play Mode tests are green.
  • Open the project in the pinned editor target (6000.3.0f1+) — compiles cleanly.
  • Run the samples: HelloWorld, Stats, TodoList, VirtualizedList, 01. Counter, 02. Greeter — all work, binders wire up correctly.
  • ViewModelDebugPanel — search / tabs / RelayCommand / bindable properties all functional.
  • MonoView / MonoBinder / MonoViewModel inspectors render via UI Toolkit; drag & drop of unassigned binders works.
  • HeaderGroup collapses inspector fields; virtual binder fields appear automatically for IView<TViewModel> members.
  • Walk through MIGRATION.md against a downstream package consumer.
  • dotnet test for Aspid.MVVM.Generators.Tests and Aspid.MVVM.Analyzers.Tests is green.
  • Trigger the release workflow against a v1.1.0-beta.1 tag — the upm-preview branch and upm-preview/1.1.0-beta.1 tag receive the expected package contents.

@VPDPersonal VPDPersonal added the status: work-in-progress Draft / not ready for review label Nov 9, 2025
@VPDPersonal VPDPersonal added the needs-changelog README/CHANGELOG update required before release label Jan 18, 2026
VPDPersonal and others added 23 commits May 30, 2026 19:28
fix(virtualized-list): guard null views/coroutine and fix listener leak
…ctness

fix(converters): guard division-by-zero and float precision in numeric converters
…-slot

fix(collections): ignore empty filter slots in OrCompositeCollectionFilter
…nteractable

fix(binders): repair custom-interactable command binder constructors
…double-conversion

fix(binders): avoid double conversion in GraphicColorComponent binders
Co-authored-by: Vladislav Panin <VPDPersonal@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix(binders): material color, sprite leak and layout dirty in visual binders
- Add CHANGELOG.ru.md and MIGRATION.ru.md (full Russian translations)
- Remove redundant editor version (6000.4.0f1) from Highlights section
- Remove package id rename entry from Changed section (already in package.json history)
docs(release): prepare 1.1.0 changelog, migration and package metadata
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace the com.aspid.fasttools git-UPM dependency with an embedded tech.aspid.fasttools package (file:)
- Update manifest.json and packages-lock.json accordingly
…mples

- Add numbered samples 03–06 demonstrating core MVVM scenarios
- Relocate the package from Assets/Aspid/MVVM/ to Packages/tech.aspid.mvvm/, mirroring the embedded tech.aspid.fasttools layout
- Rename Samples/ -> Samples~/ so samples ship via Package Manager instead of being indexed in the dev project
- Bump version to 1.1.0-beta.1 and repoint changelogUrl to the new package path
- Update path references in CLAUDE.md, Readme.md, MIGRATION.md and MIGRATION.ru.md
build(package): embed Aspid.MVVM as local UPM package (1.1.0-beta.1)
- Add embedded packages-lock entry for tech.aspid.mvvm (Unity-generated after the Assets -> Packages move)
- Add "unityRelease": "0f1" to package.json and normalize the author block
…ed package

- Readme: replace single release badge with Stable (upm) and Preview
  (upm-preview) package-json badges, matching Aspid.FastTools
- release.yml: point PACKAGE_PATH / CHANGELOG_PATH at the embedded
  Packages/tech.aspid.mvvm location (was the stale Assets/Aspid/MVVM)
- CHANGELOG (EN/RU): stamp the 1.1.0 work as [1.1.0-beta.1] so the
  workflow can extract release notes; correct the package move to
  embedded UPM, FastTools embedding, samples and badge entries
ci(release): add preview badge, fix workflow paths, refresh changelog
… 6.0+

- Add a preview-only Integration section with the upm-preview Git URL
  (branch + immutable upm-preview/<version> tag), matching Aspid.FastTools;
  stable 1.0.x still installs the usual way (Asset Store / Releases)
- Update the Unity badge from 2022.3+ to 6.0+ to match package.json
  (unity 6000.0) and the CHANGELOG, and link it to the Asset Store
docs(readme): add preview UPM install section and bump Unity badge to 6.0+
Resolve add/add conflict in .github/workflows/pr-ready-labels.yml by
taking the main version, which adds the `closed` event handler (#114)
that the release branch did not yet contain. Unblocks release PR #25.
@VPDPersonal

Copy link
Copy Markdown
Owner Author

🔍 Code review — Aspid.MVVM 1.1.0 (max-effort)

Scope. The branch diff vs main is ~6667 files, but almost all of it is the Assets→Packages migration (3615 renames) and the FastTools vendor embed. This review targets the 46 logic-bearing commits (fix/revert/perf, a 2921-line combined patch) — binders, collections, converters, virtualized-list, bindable members, editor. Renames/vendored code were excluded as noise.

Method: 9 independent finder angles → dedup → verification → gap sweep. The PR is largely solid — the Slider/Scrollbar/Toggle custom-interactable ctor fixes remove a guaranteed ArgumentOutOfRangeException, and the division-by-zero guard, the LocalizeStringEvent variable-exists inversion, the GraphicColorComponent redundant-converter removal, the ObservableList.OnMove (RemoveAt+Insert), and the culture round-trip were all verified correct. 13 issues remain, below.

Note: inline comments couldn't be anchored on the migration diff, so findings are consolidated here with path:line.


🔴 Correctness

1. CollectionViewModelBinder.OnMove — wrong display order for moves of distance > 1
Aspid.MVVM/Packages/tech.aspid.mvvm/StarterKit/Unity/Runtime/Binders/Collections/Collections/ViewModel/CollectionViewModelBinder.cs:95
This is a positional binder (_views[i] shows Collection[i], per OnAdded/OnReplace), but OnMove only swaps the sibling indices of two transforms and never re-initializes the intermediate slots. Move(0→2) on [A,B,C][B,C,A] renders [C,B,A]. Adjacent swaps (|old−new|==1) work by coincidence; any longer move desyncs the views from the collection. The sibling ObservableListViewModelBinder.OnMove got the correct fix; this layer did not.

2. CollectionViewModelBinder — granular Add/Remove do a full O(N) reset+replay per item, plus unbounded _views[index]
CollectionViewModelBinder.cs:63-85 (and :47)
The PR enables granular dispatch (Add/Remove were previously a no-op break). A single Add now runs OnReset(); OnAdded(Collection) — rebuilding every view (O(N) per change, O(N²) for N appends). OnAdded(IReadOnlyCollection) indexes _views[index] with no bound check, so growing the collection past _views.Length throws IndexOutOfRangeException where the Add was previously ignored.

3. ImageSpriteBinder / ImageSpriteMonoBinderTarget.sprite left dangling to a destroyed Sprite after unbind
StarterKit/Unity/Runtime/Binders/Images/Sprite/ImageSpriteBinder.cs:58 (+ Mono twin)
OnUnbound destroys _createdSprite but doesn't reset Target.sprite; the base TargetBinder doesn't reset the property on unbind either, so the Image references a destroyed Sprite. If the Image outlives the unbind (pooling/rebind) it renders a "missing" sprite or errors on .sprite access. Fix: have OnUnbound call SetValue((Texture2D?)null) (it already both destroys and clears).

🟡 Performance / behavior

4. RendererMaterialColorSwitcherBinder.SetValueMaterial[] allocation + material instancing on every value change
StarterKit/Unity/Runtime/Binders/Renderers/MaterialsColor/RendererMaterialColorSwitcherBinder.cs:52
Switching from Target.material to Target.materials (foreach) allocates a fresh Material[] and instances the shared materials on every SetValue (GC churn + broken batching). Consistent with the other renderer-color binders, but the switcher triggers it per value. Prefer caching the array once or a MaterialPropertyBlock.

5. NumberToBoolConverter — threshold _value stays float while the input was widened to double
StarterKit/Runtime/Converters/Bools/NumberToBoolConverter.cs:80 (field _value:24)
The compare now runs in double, but the configured threshold is stored as float. For int/long with a threshold above 2²⁴ the threshold is float-rounded (16_777_217f16_777_216), giving a wrong result and only half-achieving the "compare exactly" goal stated in the XML doc. Narrow edge case.

6. OrCompositeCollectionFilter — empty/all-null filter now hides all items (was: show all)
StarterKit/Runtime/Collections/Filters/Composites/OrCompositeCollectionFilter.cs:31
.Where(p => p is not null).Any(...) returns false for an empty sequence. This is the mathematically correct OR-identity (consistent with And) and fixes the prior "one empty slot forces show-all" bug — but a fully-unconfigured Or filter now silently hides the whole list. Likely intended; please confirm no serialized empty Or filters rely on the old behavior.

7. VirtualizedList.OnMove<= _views.Length where siblings use <
StarterKit/Unity/Runtime/Components/UI/VirtualizedList.cs:317
OnAdded/OnRemoved gate on < _views.Length; OnMove uses <=. Off-by-one → one extra full Refresh() at the window boundary. Refresh() is always safe, so this is a perf/consistency nit, not a crash.

🔵 Altitude / duplication (maintenance cost)

8. CollectionViewModelBinder.cs:63-85 — four byte-identical granular hook bodies (OnReset(); OnAdded(Collection)). This reset-and-replay should be a non-abstract default on CollectionBinderBase, not overridden 4× per subclass. (Fixing it here is the right depth for findings 1–2.)

9. SliderCommandBinder.cs:44 (+ Scrollbar/Toggle + Mono twins) — the _interactableMode/SetInteractableMode/ApplyCanExecute/custom-interactable-ctor machinery is duplicated ~24×. That's exactly why the ctor bug recurred identically in each family and had to be fixed file-by-file. Extract an InteractableCommandBinderBase<T>.

10. ToCultureStringExtensions.cs:20 — the PR added ToCultureInfo(mode) but the 5–6 ToCultureString overloads still each re-implement the same mode → CultureInfo switch. Each collapses to number.ToString(mode.ToCultureInfo()); otherwise a new CultureInfoMode value must be added in ~7 switches.

11. ObservableListViewModelBinder.cs:149 — this binder and its Mono twin are ~100 lines of byte-identical view bookkeeping (including the duplicated OnMove fix) with no shared base. Every fix must be made twice; one twin can silently keep a bug.

12. ImageSpriteBinder.cs:44 — the Texture2D→Sprite create/destroy/cleanup logic is duplicated across the two twins and across SetValue/OnUnbound. The dangling-sprite fix (finding 3) will likewise need applying in two places unless a shared helper owns the sprite lifecycle.

13. VirtualizedList.cs:284 — batch OnAdded/OnRemoved(IReadOnlyList) unconditionally call full Refresh() (marked TODO Optimize) even when the changed range is outside the visible window, while the single-item overloads already gate on viewIndex.


Automated max-effort review. Findings 1–3 are the ones worth fixing before release; the rest are lower-severity or refactors.

Fixes surfaced by the max-effort review of PR #25 (Aspid.MVVM 1.1.0):

- CollectionViewModelBinder: rebuild positional views on Move (the sibling-index swap was wrong for moves of distance > 1) and guard the view array against out-of-range; dedup the four granular hooks into RebuildFromCollection.
- ImageSpriteBinder/Mono: clear Image.sprite on unbind so it no longer references the destroyed generated sprite; share a CreateSprite helper.
- RendererMaterialColorSwitcherBinder: cache the materials array to drop the per-value Material[] allocation.
- OrCompositeCollectionFilter: an all-empty filter now passes all items (consistent with And), implemented allocation-free.
- VirtualizedList: OnMove window check uses < (matching OnAdded/OnRemoved); window-gate batch add/remove instead of unconditional Refresh.
- ToCultureStringExtensions: delegate the overloads to the existing ToCultureInfo helper.
- ObservableListViewModelBinder twins: extract shared view-list bookkeeping into ObservableListViewModelBinderHelper.

Authored without an in-Editor Unity compile; verify by compiling in Unity.
Address review feedback on this PR:

- Move ObservableListViewModelBinderHelper and the sprite CreateSprite helper into their own files (ObservableListViewModelBinderHelper.cs, SpriteBinderHelper.cs) instead of living inside a binder class.
- Call base.OnUnbound() in RendererMaterialColorSwitcherBinder — the override added earlier in this PR was silently dropping the base-class cleanup; also add it to ImageSpriteBinder/Mono to follow the codebase convention.
- Guard CollectionViewModelBinder.OnReplace against an out-of-range index, mirroring the OnAdded guard.
- Add missing trailing newlines on touched files.
fix: resolve code-review findings in binders and collections
@VPDPersonal VPDPersonal merged commit 9522990 into main Jun 7, 2026
1 check passed
@github-actions github-actions Bot removed the status: needs-review Ready for review label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI / build pipeline area: editor Editor-only code area: runtime Runtime / player code area: samples Sample projects breaking-change Backwards-incompatible change requiring major version bump dependencies Dependency or submodule update type: ci CI / build pipeline changes type: documentation Documentation only type: feature New feature or capability type: refactor Internal restructuring without behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants