Skip to content

[FIX] Fix nested array isolation in Observer constructor#58

Merged
willeastcott merged 1 commit intomainfrom
nested-arrays
Jan 10, 2026
Merged

[FIX] Fix nested array isolation in Observer constructor#58
willeastcott merged 1 commit intomainfrom
nested-arrays

Conversation

@willeastcott
Copy link
Contributor

@willeastcott willeastcott commented Jan 10, 2026

Fixes playcanvas/editor#684

Overview

The Observer class was not properly isolating nested arrays from source data, causing issues when the source data was modified externally (e.g., by ShareDB during real-time collaboration).

The Problem

When creating an Observer from data containing nested arrays (like [[1,2,3], [4,5,6]]), the inner arrays were not being copied - only the outer array was shallow copied. This meant the Observer and the source data shared references to the same inner arrays.

In the Editor, this caused script attributes with array types (e.g., vec3[]) to fail live-updating in the Launch tab:

  1. User changes a vec3 array attribute in the Inspector
  2. ShareDB receives the operation and updates its document data
  3. Because the Observer shared the same inner array reference, its data was also modified
  4. When observer.set() was called, the value was already the same, so no *:set event fired
  5. The Launch tab never received the update

The Fix

  • Added deepCopyArray() utility function in utils.ts (alongside existing arrayEquals)

  • Fixed line 205 in _prepare() which was calling .slice(0) but discarding the result:
    // Before (bug)
    target._data[key][i].slice(0);

    // After (fixed)
    target._data[key][i] = deepCopyArray(target._data[key][i]);

3 unit tests have been added. All fail without the fix but now pass.

The original Editor issue is confirmed fixed.

@willeastcott willeastcott self-assigned this Jan 10, 2026
@willeastcott willeastcott added the bug Something isn't working label Jan 10, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a bug where nested arrays were not properly isolated in the Observer constructor, causing issues with real-time collaboration via ShareDB. When source data was modified externally, the Observer's internal data was inadvertently modified as well, preventing change events from firing correctly.

Changes:

  • Added deepCopyArray() utility function to recursively copy nested arrays
  • Fixed line 205 in _prepare() method to use deepCopyArray() instead of the buggy .slice(0) call
  • Added comprehensive test coverage for nested array isolation scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/utils.ts Added deepCopyArray() utility function with proper documentation for deep copying nested arrays
src/observer.ts Fixed nested array isolation bug in _prepare() method by using deepCopyArray() instead of discarding .slice(0) result
test/observer.test.mjs Added comprehensive test suite covering nested array isolation, including the exact bug scenario from issue #684

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@willeastcott willeastcott merged commit 3ec9f4f into main Jan 10, 2026
9 checks passed
@willeastcott willeastcott deleted the nested-arrays branch January 10, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Live updating of script attribute arrays don't work

2 participants

Comments