Skip to content

[FIX] Fix ScriptComponent clone not handling awaiting scripts - #8264

Closed
willeastcott wants to merge 1 commit into
mainfrom
script-bug
Closed

[FIX] Fix ScriptComponent clone not handling awaiting scripts#8264
willeastcott wants to merge 1 commit into
mainfrom
script-bug

Conversation

@willeastcott

@willeastcott willeastcott commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

Description

When cloning an entity with a ScriptComponent, scripts in an "awaiting" state were not being properly included in the clone's script order.

Bug

The cloneComponent method was iterating over _scriptsIndex using for...in, but incorrectly checking properties on the string key instead of the script entry object:

for (const key in entity.script._scriptsIndex) {
    if (key.awaiting) {  // ❌ key is a string, key.awaiting is always undefined
        order.splice(key.ind, 0, key);
    }
}

Since key is a string (the property name), key.awaiting was always undefined (falsy), making this code block effectively dead code.

Fix

Access the actual script entry object stored at that key:

for (const key in entity.script._scriptsIndex) {
    const scriptEntry = entity.script._scriptsIndex[key];
    if (scriptEntry.awaiting) {  // ✅ correctly checks the script entry
        order.splice(scriptEntry.ind, 0, key);
    }
}

Fixes #2796

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

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 fixes a bug in the ScriptComponentSystem.cloneComponent method where scripts in an "awaiting" state (scripts that are referenced but not yet registered) were not being properly included in the cloned entity's script order due to incorrectly accessing properties on a string key instead of the script entry object.

  • Fixed the for...in loop to correctly retrieve and check the script entry object instead of checking properties on the string key
  • Now properly handles awaiting scripts by accessing scriptEntry.awaiting and scriptEntry.ind instead of key.awaiting and key.ind

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

Comment on lines +124 to +126
const scriptEntry = entity.script._scriptsIndex[key];
if (scriptEntry.awaiting) {
order.splice(scriptEntry.ind, 0, key);

Copilot AI Dec 19, 2025

Copy link

Choose a reason for hiding this comment

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

The fix correctly handles awaiting scripts during cloning, but there is no test coverage for this scenario. Consider adding a test that verifies cloning an entity with a ScriptComponent that has scripts in the "awaiting" state (i.e., scripts that are referenced but not yet registered). This would ensure the fix works as intended and prevent future regressions.

Copilot uses AI. Check for mistakes.
@mvaligursky

Copy link
Copy Markdown
Contributor

Closing in favour of #9200.

The diagnosis here is exactly right, but the one-line dereference is not enough on its own — it makes entity.clone() throw. Restoring the name into order without also adding it to scripts means initializeComponentData hits:

component.create(data.order[i], {
    enabled: data.scripts[data.order[i]].enabled,   // undefined.enabled
TypeError: Cannot read properties of undefined (reading 'enabled')

which is a worse regression than the silent drop it fixes. #9200 adds the scripts entry (from _scriptsData, the same place the deferred registry sweep reads the attributes from), plus a running offset on the splice — the stored ind is the count of instantiated scripts at declaration time, so consecutive awaiting scripts share an index and splicing each at the raw value reverses them. It comes with tests covering all three.

Thanks @willeastcott — the analysis in this PR is what pointed at the rest of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScriptComponent clone awaiting

3 participants