-
Notifications
You must be signed in to change notification settings - Fork 103
[WIP] Cancelling texture selection in material editor unsets the texture instead #683
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
Conversation
Co-authored-by: neph1 <7988802+neph1@users.noreply.github.com>
Co-authored-by: neph1 <7988802+neph1@users.noreply.github.com>
Co-authored-by: neph1 <7988802+neph1@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Tested and it works well. Just check my review comment
jme3-materialeditor/src/com/jme3/gde/materials/multiview/widgets/TexturePanel.java
Outdated
Show resolved
Hide resolved
…ts/TexturePanel.java
There was a problem hiding this comment.
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 fixes a bug where cancelling texture selection in the material editor would unset the current texture instead of preserving it. The fix ensures the texture browser dialog initializes with the current texture and only modifies the property when the user makes an explicit selection.
- Moved the property clearing logic to only execute after determining the user's choice
- Added proper initialization of the texture editor with the current texture before showing the dialog
- Enhanced the logic to distinguish between texture selection, "No Texture" selection, and dialog cancellation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
TexturePanel.java | Fixed the texturePreviewMouseClicked method to preserve texture state on cancellation |
TexturePanelTest.java | Added test cases to verify cancellation and "No Texture" selection behaviors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
jme3-materialeditor/src/com/jme3/gde/materials/multiview/widgets/TexturePanel.java
Outdated
Show resolved
Hide resolved
…ts/TexturePanel.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Will merge soon, unless there are objections |
TexturePanel.texturePreviewMouseClicked()
,property.setValue(EMPTY)
is called immediately before showing the dialog, and if the user cancels, the texture isn't restoredTexturePanel.java
andTextureBrowser.java
TexturePanelTest.java
with comprehensive test coverageImproved Changes Made:
Enhanced
TexturePanel.texturePreviewMouseClicked()
method:Updated test cases to reflect the improved logic
The improved fix ensures that:
This approach is more robust and follows the pattern used in other parts of the codebase (like the terrain editor).
Fixes #674.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.