-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
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
Save color palette as resources to reuse later #91604
Conversation
99416da
to
8ad7da9
Compare
This should use icons available at runtime, not editor icons, there are some appropriate ones available, and otherwise they should be added, this shouldn't depend on the editor |
7c37110
to
0839503
Compare
I approve of allowing saving the color pallete from a usability team perspective. |
Update:
@AThousandShips What do you mean when you say |
Icons available at runtime are those which aren't editor specific, meant in running projects not running the program itself sorry for confusion |
0839503
to
6084096
Compare
04ebe85
to
acdd5f8
Compare
Sorry for the annoying force push. I wasn't familiar with the TTR part. I need one more favor. I couldn't find the correct icon to use in the function |
If it isn't in the default theme But I think the normal folder icon is sufficient Gonna test this one probably tomorrow but code looks good, got some final remarks I'll add when I've tested it out |
I wonder if it should be possible to give a name to each color, and have the name displayed in a tooltip when hovering the color in the ColorPicker's swatch list. It's common for designers to assign names to colors to make slightly different colors easier to distinguish from each other, or even to refer to them in natural language. If I refer to "lime 800" from Tailwind's color palette, you can immediately know it's This could be left to a future PR, but we should make sure this feature can be implemented in the future without breaking compatibility (i.e. without changing the data structure in the resource). |
768da7a
to
10f2235
Compare
Tested and it works as expected, however I don't think the list of resources allowed makes sense, it should probably just limit to Edit: correction, it shouldn't even be that, it should be some binary format dedicated to this, it doesn't actually store a |
I suggest using ConfigFile to save palettes as opposed to FileAccess, so you have a text format that is friendly to version control. I'd also give them a custom file extension so they can be targeted more specifically, e.g. A similar approach is used for script editor syntax themes, which use a |
feca7e7
to
1e0de24
Compare
2a80ca0
to
987a844
Compare
@Calinou Would you like to take a look to see if this PR is satisfactory as is? |
Also is it possible to decrease the swatch colors size by 25% at least so more colors can fit in a row |
That's handled by the editor theme isn't it? |
@tokengamedev @timshannon Look at the code: the size is fixed. There is no way to change the size of the preset. godot/scene/gui/color_picker.cpp Lines 793 to 797 in 987a844
|
Let's think about that in another PR if necessary. |
3b2d22b
to
dc34d4b
Compare
@Calinou Could you spend some time reviewing my PR again? |
dc34d4b
to
e7a32ca
Compare
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.
Tested locally, it works as expected.
I noticed a few UI issues though:
- When using Quick Load, the color picker closes, so you can't see your palette being loaded (it still loads correctly when you reopen the color picker). This does not happen when using Load - the color picker stays open as expected.
- The button in the dropdown is Save, but it always displays a file dialog even if a color palette file was already loaded. In this case, it should be renamed to Save As.... A Save button that overwrites the existing loaded palette could also be added above Save As... (this button should be disabled if no palette is loaded).
@Calinou Yes, I noticed the problem with Quick Load when developing this PR. However, I cannot resolve this issue because Godot doesn't support having two exclusive dialogs at the same time in one node. This means the EditorNode already has the As for the Save As option, I will add it when I return home from work. |
e7a32ca
to
53d23e1
Compare
@Calinou I tried to move the Quick Load dialog to the color picker to avoid the limitation mentioned above. However, I realized that someone had told me I cannot include editor code in the core code, if I remember correctly. As for your first point, I still cannot resolve it, but I have implemented the second point. Please review it again when you have time. |
53d23e1
to
c7bdd11
Compare
Sorry for the delay. I've tested the latest revision of this PR, but I frequently get a freeze when adding some colors to the swatches (even without saving the palette). This occurs both in a standalone ColorPicker and ColorPickerButton. Testing project: test_color_picker.zip |
Co-authored-by: Micky <66727710+Mickeon@users.noreply.github.com>
c7bdd11
to
e6a49ab
Compare
Could you upload a video showing the issue? I just rebased the code and tested it, but I could not observe the described behavior. Maybe I was just careless and didn't pay enough attention to the details, overlooking something really important. |
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've tested the rebased PR and it works great now. The freezing issue may have been unrelated to this PR and was perhaps due to the state of master
back in October.
I took a look at the code and it seems good to me.
Thanks! |
Close godotengine/godot-proposals#7946
The lack of a palette library slows down development time because swatches contain many colors that may not match the mood an artist is trying to achieve. This can hinder their workflow as they search for the right color within a large set of mostly irrelevant options.
Untitled.video.-.Made.with.Clipchamp.mp4