Skip to content

Implement GaeaGraph deep duplication and add duplicate button#431

Merged
BenjaTK merged 24 commits intogaea-godot:2.0from
cullumi:GaeaData-Duplication-Fix
Nov 3, 2025
Merged

Implement GaeaGraph deep duplication and add duplicate button#431
BenjaTK merged 24 commits intogaea-godot:2.0from
cullumi:GaeaData-Duplication-Fix

Conversation

@cullumi
Copy link
Collaborator

@cullumi cullumi commented Sep 10, 2025

Deep Duplication

Fixes #429 , prerequisite for #425 (enables decoupling of demos and tests).

Motivation

It turns out Godot can't properly duplicate GaeaGraph resources, because deep duplication doesn't work on custom sub-resources. The end result is GaeaGraphs that have empty or broken resource lists.

Update for Godot 4.5

Deep duplication of sub-resources is now possible. Regardless, it isn't directly supported in the context menu. Additionally, not every part of a GaeaGraph needs to be duplicated, only the parts that are saved to files:

  • _connections
  • _node_data
  • _parameters
  • save_version

Solution

As a solution, I've added a duplicate button to Gaea's panel, which triggers a custom _duplicate method on the GaeaGraph class, then saves the copy made to some target file path.

Also

This PR includes a number of other fixes and tests.

Fixes

Fixes in this PR include:

  • The Gaea panel now correctly loads GaeaGraphs when they are assigned to a GaeaGenerator without needing to reload the scene or the editor.

Tests

Added graph testing:

  • test_assign_to_generator - Assign a GaeaGraph to a GaeaGenerator, then run the generator.
  • test_duplicate - Simply attempt to duplicate a GaeaGraph.
  • test_assign_duplicate - Duplicate a GaeaGraph, assign it to a GaeaGenerator, then run the generator.

@cullumi cullumi added the ✨ enhancement New feature or request label Sep 10, 2025
@cullumi cullumi added this to the v2.0.0-stable milestone Sep 10, 2025
@cullumi cullumi self-assigned this Sep 10, 2025
@cullumi cullumi linked an issue Sep 10, 2025 that may be closed by this pull request
@cullumi
Copy link
Collaborator Author

cullumi commented Sep 10, 2025

Current issue is that a pre-existing Gaea Graph (not made from duplication) is having all of it's connections cleared upon loading it in the scene. It's unclear why this occurs, it doesn't seem limited to this branch.

@cullumi
Copy link
Collaborator Author

cullumi commented Sep 10, 2025

I'll be adding a new test case for confirming that duplication is functioning properly independent of loading it to the scene.

@cullumi cullumi force-pushed the GaeaData-Duplication-Fix branch from 2981ff3 to be47db7 Compare October 4, 2025 03:23
@cullumi cullumi changed the title Gaea data duplication fix GaeaGraph duplicate button Oct 11, 2025
@cullumi cullumi marked this pull request as ready for review October 11, 2025 10:28
@cullumi cullumi changed the title GaeaGraph duplicate button GaeaGraph deep duplication + Duplicate button Oct 11, 2025
@cullumi cullumi requested review from BenjaTK and Zehir October 11, 2025 11:18
@cullumi
Copy link
Collaborator Author

cullumi commented Oct 11, 2025

Good chunk of solid changes are in. The duplicate button works as expected and is testable. Fixed a few issues with assigning a graph to a generator and not having the interface initialize properly, as well. Should be good to go unless y'all notice anything glaring.

@cullumi cullumi added the 🐞 bug Something isn't working label Oct 11, 2025
Copy link
Collaborator

@BenjaTK BenjaTK left a comment

Choose a reason for hiding this comment

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

Just a few changes, LGTM otherwise.

Zehir
Zehir previously requested changes Oct 13, 2025
@cullumi cullumi force-pushed the GaeaData-Duplication-Fix branch from e9a5783 to 4a3399a Compare October 14, 2025 01:29
Copy link
Collaborator

@BenjaTK BenjaTK left a comment

Choose a reason for hiding this comment

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

LGTM, might have to rebase after #437 to catch some formatting mistakes but I didn't notice anything GDLint would flag as wrong.

@Zehir
Copy link
Collaborator

Zehir commented Oct 18, 2025

Do you still need my review on this one ?

@cullumi cullumi dismissed BenjaTK’s stale review October 28, 2025 07:24

Resolved. Added assertions to duplicate and assign_to_generator tests, then deleted the assign_duplicate test as it was redundant.

@cullumi cullumi requested a review from BenjaTK October 28, 2025 07:25
@cullumi
Copy link
Collaborator Author

cullumi commented Oct 28, 2025

Aight this one should be good to go, @BenjaTK or @Zehir. Just need a final review so it can be merged. The important bits to review are in the testing script for the GaeaGraph.

Copy link
Collaborator

@BenjaTK BenjaTK left a comment

Choose a reason for hiding this comment

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

LGTM, except for a few changes.

Zehir
Zehir previously approved these changes Nov 2, 2025
Copy link
Collaborator

@Zehir Zehir left a comment

Choose a reason for hiding this comment

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

LGTM

@BenjaTK
Copy link
Collaborator

BenjaTK commented Nov 2, 2025

Fixed the conflict, hope that's fine.

@BenjaTK
Copy link
Collaborator

BenjaTK commented Nov 2, 2025

Actually now that I realize I'm an idiot, I broke my own rule about rebasing lol. If you want you could reset hard to the previous commit and then rebase from there, if not then it's fine.

@cullumi
Copy link
Collaborator Author

cullumi commented Nov 3, 2025

Actually now that I realize I'm an idiot, I broke my own rule about rebasing lol. If you want you could reset hard to the previous commit and then rebase from there, if not then it's fine.

Ha, I think it'll be fine, this PR will get squashed anyway lol

@cullumi cullumi requested a review from BenjaTK November 3, 2025 04:05
Copy link
Collaborator

@BenjaTK BenjaTK left a comment

Choose a reason for hiding this comment

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

LGTM

@BenjaTK BenjaTK merged commit 187e4eb into gaea-godot:2.0 Nov 3, 2025
5 checks passed
@BenjaTK BenjaTK changed the title GaeaGraph deep duplication + Duplicate button Implement GaeaGraph deep duplication and add duplicate button Nov 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working ✨ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicating GaeaData resource leads to a broken copy.

3 participants