Skip to content

Prevent group delete at the same time as select or when panel is up#22

Merged
RailKill merged 1 commit into
mainfrom
bugfix/prevent-delete
Aug 20, 2024
Merged

Prevent group delete at the same time as select or when panel is up#22
RailKill merged 1 commit into
mainfrom
bugfix/prevent-delete

Conversation

@RailKill

@RailKill RailKill commented Aug 19, 2024

Copy link
Copy Markdown
Collaborator

This bug was introduced by me in #19, it allowed the user to delete nodes with the Delete key while the panel is open. This causes crashes because the node is no longer there while editing properties, as the Delete key is used in text-editing as well, but will delete the node at the same time. There was also a funny way to crash the program by selecting the node and deleting it within await 0.1 seconds, so the node is no longer there when the panel wants to open.

This PR fixes the 0.1 second crash window, and prevent crashes associated with triggering the Delete hotkey while panel is open. You can still delete nodes with the 'X' button while the panel is open, this works fine and doesn't crash the program, the problem is with the delete timing with the Delete key.

Summary by Sourcery

Prevent node deletion when the side panel is open or when a node is selected and deleted within a short time frame to fix crashes.

Bug Fixes:

  • Fix crashes caused by deleting nodes with the Delete key while the side panel is open by adding checks to prevent deletion in this state.
  • Resolve a crash issue that occurred when selecting and deleting a node within 0.1 seconds, ensuring the node is valid before setting it as active.

@sourcery-ai

sourcery-ai Bot commented Aug 19, 2024

Copy link
Copy Markdown

Reviewer's Guide by Sourcery

This pull request addresses a bug introduced in a previous commit (#19) that allowed users to delete nodes with the 'Delete' key while the panel is open, causing crashes. The changes prevent node deletion when the side panel is visible or when a node is being actively edited. Additionally, it fixes a crash that occurred when selecting and deleting a node within a 0.1-second window.

File-Level Changes

Files Changes
Scripts/MonologueGraphEdit.gd Modified the _input function to prevent node deletion when the side panel is visible or a node is being actively edited
Scripts/SidePanelNodes/SidePanelNodeDetails.gd Updated the on_graph_node_selected function to set the active_graphnode only when the node is valid, not in moving mode, and is the only selected node

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

@RailKill RailKill requested a review from atomic-junky August 19, 2024 06:24

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @RailKill - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@atomic-junky atomic-junky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems good to me!

@RailKill RailKill merged commit ef4a635 into main Aug 20, 2024
@RailKill RailKill deleted the bugfix/prevent-delete branch August 20, 2024 13:02
@RailKill RailKill mentioned this pull request Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants