Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5be70a6cf1
ℹ️ 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".
| for (const [variantName, { variant }] of conflictedVariants) { | ||
| replacingVariants.set(variantName, variant); | ||
| } |
There was a problem hiding this comment.
Honor variant selection when preparing replacements
importAssets copies every conflicted variant into replacingVariantsByObjectType without checking isSelected, so unchecking a variant in the conflict UI does not prevent it from being replaced. In projects with locally edited variants, this silently overwrites changes even when the user explicitly chose to keep the local version.
Useful? React with 👍 / 👎.
newIDE/app/src/EventsFunctionsExtensionsLoader/Storage/BrowserEventsFunctionsExtensionOpener.js
Show resolved
Hide resolved
| for (const { objectFolderPath, objects } of objectsByFolder) { | ||
| const subFolder = objectTreeRoot.getOrCreateSubFolders( | ||
| objectFolderPath | ||
| ); | ||
| subFolder.objects = objects; |
There was a problem hiding this comment.
Preserve earlier objects when a folder appears twice
The tree reconstruction overwrites subFolder.objects for each folder occurrence instead of appending, so if archive entries for the same folder are non-contiguous (for example A/obj1, then A/Sub/..., then A/obj2), previously collected objects are lost. This causes valid exported assets to disappear from the import selection and never be imported.
Useful? React with 👍 / 👎.
| readEventsFunctionExtensionFile: ( | ||
| filepath: any | ||
| ) => Promise<SerializedExtension>, | ||
| chooseAssetPackFile: () => Promise<string>, |
There was a problem hiding this comment.
Typing seems wrong no? That should be a File?
There was a problem hiding this comment.
Actually you can exceptionally put any here (in practice, a templated type would be better, but well)
| } else { | ||
| const newVariants = getOrCreate( | ||
| newVariantsByObjectType, | ||
| objectType, | ||
| () => new Map<string, any>() | ||
| ); | ||
| newVariants.set(variantName, serializedVariant); | ||
| } | ||
| } else { | ||
| const newVariants = getOrCreate( | ||
| newVariantsByObjectType, | ||
| objectType, | ||
| () => new Map<string, any>() | ||
| ); | ||
| newVariants.set(variantName, serializedVariant); | ||
| } |
There was a problem hiding this comment.
Nitpicking but this is verbose so worth doing it I think:
-
Make a helper "getVariantIfExisting(project, objectType, variantName)"
-
Do:
const variant = getVariantIfExisting(project, objectType, variantName);
if (variant) ...
else {
const newVariants = getOrCreate(
newVariantsByObjectType,
objectType,
() => new Map<string, any>()
);
newVariants.set(variantName, serializedVariant);
}
There was a problem hiding this comment.
It can't be moved outside the if because it would add empty values in the map.
I agree it's verbose, but the helper would have 4 parameters which would be a bit opaque.
* Send context of the conversation to improve AI asset search * Add a short description for all extensions and dimension field (2D, 3D, 2D/3D or n/a) (4ian#8303) Only show in developer changelog * Add missing support for short description/dimension Don't show in changelog * Fix being able to rename a layer with shortcut (4ian#8309) * Fix opening the 3D editor not working after launching a preview first on the web (4ian#8310) * Fix property group wrongly set when dragging a folder to the root (4ian#8312) * Fix handling of instance/locked variables (4ian#8311) - Removed the + button that was wrongly shown to add an instance variable (instance variables can only "override" the values of the variables defined in the object) - Fix paste/delete a child of structure/array of a variable that is re-defined in instances variables. Previously it was not possible to paste a variable as a child of a instance variable that was a structure/array. This is now possible (and fully supported - you can't add new variables at the root, but items inside structures/arrays can be changed). * Fix properties sometimes missing or duplicated when grouped in columns (4ian#8313) * Fix keyboard shortcuts (copy/cut/paste/delete/undo/redo) not working when editing variables * Replace old search bar by compact search bar in scene editor panels (4ian#8317) * Improve robustness of AI events generation and asset swapping * [Auto PR] Update translations (4ian#8304) This updates the translations by downloading them from Crowdin and compiling them for usage by the app. Please double check the values in `newIDE/app/src/locales/LocalesMetadata.js` to ensure the changes are sensible. Co-authored-by: 4ian <1280130+4ian@users.noreply.github.com> * Add support for ordering For Each by an expression, with optional limit (4ian#8319) - For advanced use cases where it's important to run events on each instance of an object with an ordering, the For Each can now have an ordering: an expression can be written and will be evaluated for each instance separately. The For Each will then execute for each instance in the order of the result of the expression. - It's possible to change the direction (ascending or descending) - It's also possible to give a limit (for example, 1 will allow to pick just the instance with the highest/lowest value for the expression). - Examples are provided in the UI for common use cases (maximum value of a variable, health points, ammo, distance to another object...) * Update cordova config for new xcode/ios versions (4ian#8321) Only show in developer changelog * Fix tileset scrollbar area changing selection (4ian#8324) Fix 4ian#8316 * Improve details of center/origin when an object is created by the AI (4ian#8322) * Add a simplified version of the Anchor behavior editor (4ian#8325) * [Auto PR] Update translations (4ian#8323) * Bump newIDE version * Fix size calculation for variants Don't show in changelog * [Auto PR] Update translations (4ian#8328) * Fix new group name to ensure no usage of forbidden characters (4ian#8329) * Add storybook stories for Anchor behavior editors (4ian#8330) Do not show in changelog * Fix brackets showing in the top right corner of the scene editor (4ian#8332) Don't show in changelog * Fix Scene Editor panels not resizing as small as possible (4ian#8331) * Allow to import asset pack files (GDO) (4ian#8296) * Explain the difference between built-in and external extensions in the documentation (4ian#8335) - Don't show in changelog * Fix to avoid objects to move when the gizmo dragging point is clicked (4ian#8334) * Tweak the delay before moving an object with gizmo in the 3D editor * Fix event generation errors not properly reported * Allow multiple deletions in a single operation for generated events * Persist properties panel scroll position for instances and objects (4ian#8337) --------- Co-authored-by: Florian Rival <Florian.rival@gmail.com> Co-authored-by: Florian Rival <florian@gdevelop.io> Co-authored-by: Clément Pasteau <4895034+ClementPasteau@users.noreply.github.com> Co-authored-by: D8H <Davy.Helard@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: 4ian <1280130+4ian@users.noreply.github.com> Co-authored-by: LuniMoon <103995399+LuniMoon@users.noreply.github.com>
* Send context of the conversation to improve AI asset search * Add a short description for all extensions and dimension field (2D, 3D, 2D/3D or n/a) (4ian#8303) Only show in developer changelog * Add missing support for short description/dimension Don't show in changelog * Fix being able to rename a layer with shortcut (4ian#8309) * Fix opening the 3D editor not working after launching a preview first on the web (4ian#8310) * Fix property group wrongly set when dragging a folder to the root (4ian#8312) * Fix handling of instance/locked variables (4ian#8311) - Removed the + button that was wrongly shown to add an instance variable (instance variables can only "override" the values of the variables defined in the object) - Fix paste/delete a child of structure/array of a variable that is re-defined in instances variables. Previously it was not possible to paste a variable as a child of a instance variable that was a structure/array. This is now possible (and fully supported - you can't add new variables at the root, but items inside structures/arrays can be changed). * Fix properties sometimes missing or duplicated when grouped in columns (4ian#8313) * Fix keyboard shortcuts (copy/cut/paste/delete/undo/redo) not working when editing variables * Replace old search bar by compact search bar in scene editor panels (4ian#8317) * Improve robustness of AI events generation and asset swapping * [Auto PR] Update translations (4ian#8304) This updates the translations by downloading them from Crowdin and compiling them for usage by the app. Please double check the values in `newIDE/app/src/locales/LocalesMetadata.js` to ensure the changes are sensible. Co-authored-by: 4ian <1280130+4ian@users.noreply.github.com> * Add support for ordering For Each by an expression, with optional limit (4ian#8319) - For advanced use cases where it's important to run events on each instance of an object with an ordering, the For Each can now have an ordering: an expression can be written and will be evaluated for each instance separately. The For Each will then execute for each instance in the order of the result of the expression. - It's possible to change the direction (ascending or descending) - It's also possible to give a limit (for example, 1 will allow to pick just the instance with the highest/lowest value for the expression). - Examples are provided in the UI for common use cases (maximum value of a variable, health points, ammo, distance to another object...) * Update cordova config for new xcode/ios versions (4ian#8321) Only show in developer changelog * Fix tileset scrollbar area changing selection (4ian#8324) Fix 4ian#8316 * Improve details of center/origin when an object is created by the AI (4ian#8322) * Add a simplified version of the Anchor behavior editor (4ian#8325) * [Auto PR] Update translations (4ian#8323) * Bump newIDE version * Fix size calculation for variants Don't show in changelog * [Auto PR] Update translations (4ian#8328) * Fix new group name to ensure no usage of forbidden characters (4ian#8329) * Add storybook stories for Anchor behavior editors (4ian#8330) Do not show in changelog * Fix brackets showing in the top right corner of the scene editor (4ian#8332) Don't show in changelog * Fix Scene Editor panels not resizing as small as possible (4ian#8331) * Allow to import asset pack files (GDO) (4ian#8296) * Explain the difference between built-in and external extensions in the documentation (4ian#8335) - Don't show in changelog * Fix to avoid objects to move when the gizmo dragging point is clicked (4ian#8334) * Tweak the delay before moving an object with gizmo in the 3D editor * Fix event generation errors not properly reported * Allow multiple deletions in a single operation for generated events * Persist properties panel scroll position for instances and objects (4ian#8337) * Fix For Each rendering when disabled and default values * Rename "any order" to "default order" for For Each events * Fix npm commands not working with projects on a different disk on Windows (4ian#8345) * Improve events search UI with Match Case/Filters icon --------- Co-authored-by: Florian Rival <Florian.rival@gmail.com> Co-authored-by: Florian Rival <florian@gdevelop.io> Co-authored-by: Clément Pasteau <4895034+ClementPasteau@users.noreply.github.com> Co-authored-by: D8H <Davy.Helard@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: 4ian <1280130+4ian@users.noreply.github.com> Co-authored-by: LuniMoon <103995399+LuniMoon@users.noreply.github.com> Co-authored-by: opaldi <2235599+opaldi@users.noreply.github.com>
No description provided.