Skip to content

Fix unreliable Duplicate button in My Assets#11485

Merged
riknoll merged 4 commits into
microsoft:masterfrom
KohanMathers:duplication-fix
Jul 23, 2026
Merged

Fix unreliable Duplicate button in My Assets#11485
riknoll merged 4 commits into
microsoft:masterfrom
KohanMathers:duplication-fix

Conversation

@KohanMathers

Copy link
Copy Markdown
Contributor

Fixes microsoft/pxt-arcade#7589

Problem

The Duplicate button in the My Assets panel would silently do nothing for:

  • Unnamed assets
  • An asset that was just renamed (unnamed -> named) in the same session
  • A brand-new asset with an auto-generated default name (e.g. a new tile named myTile24)
  • A sprite embedded directly in a block

In all cases, reloading the project made duplication work again.

Root causes

  1. isGalleryAsset() could return undefined instead of false for any asset whose id doesn't start with "sprites." and has no meta.package (tiles, temporary block-embedded assets, and others). duplicateAssetHandler/copyAssetHandler then called .toString() on that undefined, throwing synchronously before the duplicate/copy logic ever ran. This was the primary cause: a silent, uncaught exception in the click handler.
  2. Stale/type-unsafe reselection after any asset-list refresh re-selected the current asset by id only, with no type check, and no fallback when a promoted (newly-named) asset's id changed.
  3. buildAssetsAsync() cleared its rebuild flag before attempting work and had no try/catch, so any failure silently wedged asset syncing for the rest of the session with no user-visible error.
  4. pushUndo() could delete the asset being duplicated via its unconditional cleanupTemporaryAssets() call, and cloneAsset() mutated its input object in place instead of producing an independent copy.

Fix

  • isGalleryAsset() now always returns a real boolean.
  • Duplicate/edit/create now explicitly reselect the resulting asset instead of relying on implicit state carry-through.
  • buildAssetsAsync() self-heals its rebuild flag on failure, and failures now surface as a visible error notification instead of failing silently.
  • The in-flight selected asset is protected from cleanupTemporaryAssets().
  • cloneAsset() no longer mutates its input.

API notes

Two pxt.TilemapProject methods gained a new optional trailing parameter, which is non-breaking for all existing callers (all current call sites across pxtlib/webapp/pxtblocks/pxteditor already pass zero args):

  • pushUndo() -> pushUndo(protectedAssetId?: string)
  • cleanupTemporaryAssets() (protected) -> cleanupTemporaryAssets(protectedAssetId?: string)

cloneAsset()'s signature is unchanged, but its behavior changed: it previously mutated the meta object on its input asset as a side effect before returning the clone; it no longer does. Any code relying on that mutation (none found in this repo) would need to be updated, but this was an unintentional side effect of what's documented/named as a pure clone function.

Fix was manually verified from a

cd ../pxt-arcade
pxt link ../pxt
pxt serve

@KohanMathers

Copy link
Copy Markdown
Contributor Author

@jwunderl would you mind taking a look when you get a chance? :)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a set of reliability issues in the Asset Editor (“My Assets”), primarily around duplicate/copy/edit/create flows failing silently due to type/boolean edge cases and rebuild/selection state getting wedged or stale.

Changes:

  • Makes isGalleryAsset() always return a strict boolean to prevent click-handler exceptions from .toString() on undefined.
  • Improves asset selection resiliency across asset-list refreshes and explicitly reselects newly edited/duplicated/created assets.
  • Hardens asset rebuild logic (buildAssetsAsync) to restore the rebuild flag on failure, and adds user-visible error notifications for common asset operations.
  • Extends TilemapProject.pushUndo() / cleanupTemporaryAssets() with an optional “protected asset id”, and makes cloneAsset() avoid mutating its input.
Show a summary per file
File Description
webapp/src/package.ts Adds failure recovery for the asset rebuild flag in buildAssetsAsync().
webapp/src/components/assetEditor/store/assetEditorReducerState.ts Ensures isGalleryAsset() returns a real boolean.
webapp/src/components/assetEditor/store/assetEditorReducer.ts Improves selection retention by reselecting with (type,id) and preserving selection when not present in refreshed user-asset list.
webapp/src/components/assetEditor/assetSidebar.tsx Protects selected/in-flight assets during undo cleanup; reselects results after edit/duplicate; adds error notifications.
webapp/src/components/assetEditor/assetGallery.tsx Reselects newly created assets and surfaces create failures via error notifications.
pxtlib/tilemap.ts Adds optional protected asset id to undo cleanup; prevents cloneAsset() from mutating its input; avoids deleting a protected temporary image.

Review details

  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread webapp/src/components/assetEditor/assetGallery.tsx Outdated
Comment thread webapp/src/components/assetEditor/assetSidebar.tsx Outdated
@KohanMathers

Copy link
Copy Markdown
Contributor Author

@copilot's comments resolved and microsoft:master merged back in so the PR branch is up to date

@KohanMathers

Copy link
Copy Markdown
Contributor Author

Hey @riknoll - just waiting on that review :)

@riknoll riknoll left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@riknoll
riknoll enabled auto-merge July 23, 2026 18:22
@riknoll
riknoll added this pull request to the merge queue Jul 23, 2026
Merged via the queue into microsoft:master with commit ea060fd Jul 23, 2026
12 checks passed
@KohanMathers
KohanMathers deleted the duplication-fix branch July 23, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

My Assets - Duplicate not consistently working

3 participants