Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Orama-Interactive#958 (reset custom brush when switch another tool) #1078

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

alikin12
Copy link
Contributor

@alikin12 alikin12 commented Aug 13, 2024

This should fix issue #958 (reset custom brush when switch another tool) by removing the childs safely from BrushPopup when project data is cleared.

@OverloadedOrama
Copy link
Member

Hello, thank you for the contribution! I'm not sure I understand what this change accomplishes, or how it's related to issue #958. I am trying to switch between projects with and without your change and I do not notice anything different. Can you explain how this fixes an issue?

@alikin12
Copy link
Contributor Author

Hello. First of all I want to thank you for your great work and the effort you and all other people who are involved in this project put into pixelorama. It's a very nice tool.

Regarding the issue and my fix I try to explain how to reproduce the issue and how the change fixes it:

Reproducing issue:

  1. Create new brush
  2. Save the project
  3. Open the project a second time (or close the project and reopen it. Important is that the project is reloaded)
  4. In the reloaded project: Select pencil tool and set brush to the new created brush. The brush should now be correctly selected
  5. Change to any other tool
  6. Change back to pencil tool. Now the self created brush is no more selected but the default Pixel brush

All this happens only when project was saved and reloaded. It does not happen without the reloading step.

Fix:
The issue appears because when a project is loaded, the brushes are added in the open_pxo_file method by add_project_brush. After that the change_project method is called which should clear all brushed and repopulate it again with the project ones. Without the fix the old brushes are not cleared directly (I think because queue_free() takes effect only on the next frame) and when the new brushes are added, the index which is set by get_index() is set to the wrong value as the old brushes still exist and the index has now an offset of the old brushs count. When switching the tool from another tool back to pencil, the brush with the wrong (not existing) index is tried to selected. As the child with this index does not exist, it fails and the default brush is selected.

Hope that helps :)

Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

Oh okay that makes perfect sense! Thank you very much for the detailed explanation and the solution to the problem!

@OverloadedOrama OverloadedOrama merged commit f6f40e0 into Orama-Interactive:master Aug 15, 2024
4 checks passed
@alikin12 alikin12 deleted the issue_958 branch September 3, 2024 17:36
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