Skip to content

Yet another try to fix the crashes#27

Merged
gensyn merged 3 commits into
mainfrom
fix
Jul 3, 2026
Merged

Yet another try to fix the crashes#27
gensyn merged 3 commits into
mainfrom
fix

Conversation

@gensyn

@gensyn gensyn commented Jul 3, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings July 3, 2026 14:02
@gensyn gensyn changed the title Cleaned up code for image support Yet another try to fix the crashes Jul 3, 2026
@gensyn

gensyn commented Jul 3, 2026

Copy link
Copy Markdown
Owner Author

@copilot Please fix the tests I broke in this branch.

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 attempts to prevent crashes when changing domains by keeping references to combo-row items alive while clearing the entity combo box, likely to avoid premature destruction/GC issues in the underlying GTK/ListModel objects.

Changes:

  • Keep entity combo items referenced in a temporary list during remove_all_items() to prevent crashes.
  • Explicitly release the temporary references after clearing the combo.

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

Comment thread actions/cores/base_core/base_core.py

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@copilot Please fix the tests I broke in this branch.

Fixed in the latest commit. The 5 tests that call on_change_domain with differing old/new domains (triggering the get_n_items path) now configure entity_combo_mock.get_n_items = Mock(return_value=0) so range() receives a proper integer. Production code in base_core.py is untouched.

Copilot finished work on behalf of gensyn July 3, 2026 14:09
@gensyn gensyn merged commit 98937b7 into main Jul 3, 2026
2 checks passed
@gensyn gensyn deleted the fix branch July 3, 2026 14:11
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.

3 participants