Display the old color in ColorPicker for easier comparison

[Calinou]

master version of #48611.

This only affects ColorPickerButton nodes that spawn a ColorPicker, not standalone ColorPickers. The old color is on the left, the new color is on the right.

This partially addresses #7366.

Preview

[YuriSizov]

In hopes for this to get merged before 3.2.3 release maybe worth resolving the conflicting file? 🙂

[Calinou]

Rebased and tested again. It still works as expected:

[YuriSizov]

I've rebased and tested it locally, works as intended. Though for a very specific case of the editor theme it may be worth to have some border around the colors, or something:

[Calinou]

@pycbouh Good catch, but I'd prefer adding the border in a separate PR. This issue is present without the change provided by this PR anyway.

[gongpha]

I think it will be better when clicking on the old color box. And the color will reset to an old one. This should help those who pick the wrong color and don't want to close a modal.

[Calinou]

Rebased and tested again:

I think it will be better when clicking on the old color box. And the color will reset to an old one. This should help those who pick the wrong color and don't want to close a modal.

This is a good idea, but I'll leave it for a future pull request.

Edit: Pull request opened: #48618

[YuriSizov]

Yep, still works both in editor and in projects. Though, I've missed this initially:

This only affects ColorPickerButton nodes that spawn a ColorPicker, not standalone ColorPickers.

Why don't we expose that for any ColorPicker? Not even as an option. The API is sound, it can be exposed to the user, I think.
We can do it in another PR, I'm just not sure why the limitation is here in the first place.

[Calinou]

Why don't we expose that for any ColorPicker? Not even as an option. The API is sound, it can be exposed to the user, I think.
We can do it in another PR, I'm just not sure why the limitation is here in the first place.

Unlike ColorPickerButtons which let you toggle the ColorPicker's visibility, standalone ColorPickers are always visible by default. Therefore, we'd need to define some API to reset the old color (or manually set it to an arbitrary value).

[YuriSizov]

Yes, and you've defined it already 🙂

void ColorPicker::set_old_color(const Color &p_color) {
	old_color = p_color;
}

[Calinou]

@pycbouh It should be easy to expose those methods, but I'd still leave it to a future PR to ensure we can get everything right (including hiding the Old Color property in the inspector when the Display Old Color property is unchecked).

[KoBeWi]

Aside from that one comment I left it looks ok.

[akien-mga]

Thanks!

[akien-mga]

This would need a dedicated PR to backport to 3.x and handle the theme and callback changes.