Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34d6527c99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const oldObjectDataList = HotReloader.resolveCustomObjectConfigurations( | ||
| oldProjectData, | ||
| oldLayoutData.objects | ||
| [...oldProjectData.objects, ...oldLayoutData.objects] | ||
| ); |
There was a problem hiding this comment.
When a scene object and a global object share the same name (this is a supported project setup), building oldObjectDataList/newObjectDataList as [...project.objects, ...layout.objects] introduces duplicate names with the global definition first. _hotReloadRuntimeSceneObjects then resolves by name using find(...), so the scene entry can be compared against the global entry, which can apply wrong data or trigger unregisterObject/recreation of live scene instances and lose runtime state during hot reload. Deduplicate by name with scene precedence before passing these arrays.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return variablesContainersList.getVariablesContainerFromVariableNameOnly( | ||
| props.value | ||
| ); |
There was a problem hiding this comment.
🟡 Missing getRootVariableName causes wrong dialog/tab for variables with child accessors
The new variablesContainer lookup at AnyVariableField.js:95 passes props.value directly to getVariablesContainerFromVariableNameOnly, but this C++ function (VariablesContainersList.cpp:173-186) checks Has(variableName) for an exact top-level match. When props.value contains child accessors (e.g., "MyVar.child"), the lookup fails and returns a badVariablesContainer with sourceType = Unknown. This means the wrong dialog opens (never LocalVariablesDialog) and the wrong tab is selected (never global). The existing getVariableSourceFromIdentifier helper at AnyVariableField.js:176 correctly uses getRootVariableName(variableName) before calling the same C++ function, showing this is an oversight.
| return variablesContainersList.getVariablesContainerFromVariableNameOnly( | |
| props.value | |
| ); | |
| return variablesContainersList.getVariablesContainerFromVariableNameOnly( | |
| getRootVariableName(props.value) | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyName( | ||
| props.value | ||
| ); |
There was a problem hiding this comment.
🟡 Missing getRootVariableName causes wrong dialog/tab for variables with child accessors
Same issue as in AnyVariableField.js: props.value is passed directly to getVariablesContainerFromVariableOrPropertyName at line 93 without extracting the root variable name. The C++ function (VariablesContainersList.cpp:160-171) checks Has(variableName) for an exact match, so "MyVar.child" won't find the container for "MyVar". The existing getVariableSourceFromIdentifier helper at AnyVariableOrPropertyField.js:150 correctly uses getRootVariableName before calling the same function.
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyName( | |
| props.value | |
| ); | |
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyName( | |
| getRootVariableName(props.value) | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyOrParameterName( | ||
| props.value | ||
| ); |
There was a problem hiding this comment.
🟡 Missing getRootVariableName causes wrong dialog/tab for variables with child accessors
Same issue as in the other two variable field files: props.value is passed directly to getVariablesContainerFromVariableOrPropertyOrParameterName at line 89 without extracting the root variable name. The C++ function (VariablesContainersList.cpp:149-158) checks Has(variableName) for an exact match. The existing getVariableSourceFromIdentifier helper at AnyVariableOrPropertyOrParameterField.js:149 correctly uses getRootVariableName before calling the same function.
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyOrParameterName( | |
| props.value | |
| ); | |
| return variablesContainersList.getVariablesContainerFromVariableOrPropertyOrParameterName( | |
| getRootVariableName(props.value) | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Uh oh!
There was an error while loading. Please reload this page.