-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
Replace old palette system with a new one #447
Replace old palette system with a new one #447
Conversation
Issues (not sure if related to undo/redo being not implemented yet): GUI is not being instantly updated when modyfying palettes loaded from the disk (not created at the current Pixelorama run). |
Thank you for testing. What operating system do you use? I see that the path for the palette is in res:// which doesn't happen to me - all imported palettes should be stored/copied to appdata. |
Windows 7. I was running this version in debug mode, right from the Godot. All I have in appdata are: a backup, config file and some logs.
I've just downloaded current official version ( |
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.
Eyo!
I tested your PR, I really like the new palette system in general. I'd like to comment some details I figured out while playing with the settings:
- When you put high values on width and height it can crash the program. For example I put 100 for both width and height, and Pixelorama froze for several seconds. When I tried to add a color, it crashed. Μaybe have a limitation as to how many colours can be added with relatively no problem in most devices?
- You can have 0 as the value for width or height, if there is no specific reason for that, why not have a minimum value of 1?
- When you select a colour from the palette and then you switch the colour from the colour picker (not the palette's colour), the palette colour remains selected. That's a bit confusing since you have a specific colour shown at the colour picker and a different one selected at the palette, so I suggest deselecting the palette colour when the user changes the colour of the colour picker.
- When the user creates a new palette it's not clear enough that a name is required so that the Ok button can be pressed. That can be fixed by adding some red text indicating that the name is missing when the field is empty
At this point all the issues kleonc and Erevoid had should be resolved. Palette size now has no impact on performance. |
Add middle click scrolling
6ce2f4f
to
5637325
Compare
Hide add/delete color buttons when no palette is displayed.
I added two small fixes based on luiq54's feedback on Discord. |
Didn't find any new issues after the changes, it's looking good in my opinion! |
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.
I think it looks good too. I've been testing it and it seems to be working well, so I'll go ahead and merge it now. Thank you!
I completely remade the palette system. Basically all features from #590 are implemented except undo/redo support.
This pull request breaks compatibility with old palettes. When I was redesigning the load/save system I decided to go with Resource format for palettes so the old JSON won't load anymore. Keeping legacy JSON related code seemed like a bad idea.
New dialogs: