prepareExport() mutates live blockList state for nop*Block types during serialization, causing inconsistent export behavior across repeated saves

[sahu-virendra-1908]

Description

While reviewing the export pipeline in Music Blocks, I noticed that prepareExport() directly mutates live block objects while serializing nop*Block types.

Location:

js/activity.js

Current implementation:

case "nopValueBlock":
case "nopZeroArgBlock":
case "nopOneArgBlock":
case "nopTwoArgBlock":
case "nopThreeArgBlock":
    myBlock.name = myBlock.privateData;
    break;

prepareExport() is expected to behave as a read-only serialization step, but this code overwrites the live runtime value of:

myBlock.name

inside the active blockList.

Because the mutation is applied directly to the canvas/runtime block object and is never restored afterward, later export or runtime operations may observe a different block name than the original nop*Block value.

This becomes especially problematic because the serialization logic itself depends on matching:

nopValueBlock
nopZeroArgBlock
nopOneArgBlock
...

during future export calls.

After the first mutation, subsequent exports may no longer enter these switch branches because the block name has already been replaced with privateData.

Expected Behavior

prepareExport() should serialize block data without mutating live runtime block objects.

The active canvas state should remain unchanged after export, auto-save, or save operations.

Current Behavior

During export, nop*Block types overwrite their live .name value with:

myBlock.privateData

This mutation persists after serialization completes.

Subsequent export operations may then:

• serialize different block names
• skip nop*Block handling logic
• produce inconsistent save/load behavior for plugin fallback blocks

Why This Matters

prepareExport() is triggered from several common workflows, including:

• auto-save
• manual save
• save-as
• play/save flows

Mutating runtime block state during serialization can therefore affect later editor behavior and repeated exports within the same session.

Root Cause

The serializer directly mutates:

myBlock.name

instead of using a temporary serialization-only value.

This causes runtime state leakage from the export pipeline back into the active canvas model.

Suggested Fix

Avoid mutating myBlock.name during serialization.

Instead, use a local serialization variable:

const serializedName =
    myBlock.privateData || myBlock.name;

and serialize that value without modifying the live block object.

Suggested Approach

case "nopValueBlock":
case "nopZeroArgBlock":
case "nopOneArgBlock":
case "nopTwoArgBlock":
case "nopThreeArgBlock": {
    const serializedName =
        myBlock.privateData || myBlock.name;

    data.push([
        blockIndexById.get(blk),
        serializedName,
        myBlock.container.x,
        myBlock.container.y,
        connections
    ]);

    continue;
}

Checklist

• I have read and followed the project's code of conduct.
• I have searched for similar issues before creating this one.
• I have provided all the necessary information to understand and reproduce the issue.
• I am willing to contribute to the resolution of this issue.

[sahu-virendra-1908]

@sum2it @walterbender @omsuneri Please review this issue

[walterbender]

It is a bit tricky. The mutation is to make an attempt to let a project with unknown blocks (perhaps a missing plugin) load without crashing. But as you observed, this changes the project (and, as in the case of the missing plugin, after the plugin is loaded, the project has changed.) Maybe the better solution would be to treat any unknown block as unknown as opposed to overwriting it. The mapping to unknown is a bit frail as well... distinguishing between arg and flow could be a bit more robust.

[sahu-virendra-1908]

Thank you for the detailed explanation, @walterbender sir

I understand now the mutation was intentional as a recovery
mechanism for missing-plugin blocks. The real problem is that
the mutation is never reversed, so:

1. After first export: block.name = privateData (original name)
2. Second export: switch no longer matches "nopValueBlock" →
   block serialized without nop* wrapper → plugin reload can
   no longer recover it

Your suggestion of treating unknown blocks as unknown consistently
is cleaner. Here is the approach I am thinking:

During export (prepareExport):

• Keep myBlock.name as-is (do NOT mutate)
• Serialize using a local variable:
  const serializedName = myBlock.privateData || myBlock.name
• This preserves the nop* wrapper in the exported JSON

During load (loadNewBlocks):

• When a nop*Block is encountered, check privateData for the
  original block name
• Attempt to resolve it from protoBlockDict
• If the plugin is now loaded → restore the real block
• If still missing → keep as nop* wrapper

For arg/flow distinction:
Currently all nop* types are treated identically. A more robust
approach would check the connection count or the original block's
known type from protoBlockDict to assign the correct
nopZeroArgBlock / nopOneArgBlock etc. rather than guessing.

Would this direction be acceptable? I can implement it if so.
I will wait for your confirmation before opening a PR.

[walterbender]

This makes sense.