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

Add pre-4.3 Editor theme color names for compatibility #89302

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Mar 8, 2024

Closes #88626

#87085 renames some Editor theme colors for consistency. This clean-up forces some Editor plugins to adapt for 4.3 (see #88626).
This PR restores the prior names as much as applicable along with the new names, as potential "aliases".

Keep in mind these colours should be removed at our earliest convenience. I wish there was a way to express this more "publicly".

@Mickeon
Copy link
Contributor Author

Mickeon commented Mar 8, 2024

Mentioning @Jowan-Spooner because they may know more than these three.

@TCROC
Copy link
Contributor

TCROC commented Mar 8, 2024

Why do we want to remove these colors at earliest convenience? Why not keep the aliases in order to maintain compatibility? I'm personally a fan of maintaining compatibility over new editions and releases.

@Mickeon
Copy link
Contributor Author

Mickeon commented Mar 8, 2024

I'm not the one you should be asking as I do not have full control on this, I just know it will happen.

It's mostly for spring-cleaning sake. Old code creeps in and accumulates over time. Major releases are a chance to clean the slate, rename things, remove leftover code, all of which is healthy for the whole codebase.
Even then, the removal of these colors is not catastrophic. Editor Themes and plugins can work without them.

@Jowan-Spooner
Copy link

This sounds good to me. I'm not certain these are the only ones tbh, but I think so. I assume we'll just have to move away from the ones that have been fully removed. A bit annoying to have such compatibility breaking, as it's a bit unneccessary work keeping your plugin working whenever godot updates. But it's already a big improvement to have names that work in old and new versions! Thank you very much.

@Mickeon
Copy link
Contributor Author

Mickeon commented Mar 9, 2024

I do not see the harm in bringing back the removed ones as well, so long as they do not interfere with existing colors, and are known to be unused by the engine ("deprecated" pretty much).
The main holdback is, I am struggling to find the closest equivalents with the "before and after" images provided in the issue, whether that would be by color, or by name, or by what they were actually used for.

@Jowan-Spooner
Copy link

Just curious, what is the state of this, or more precisely what needs to happen for this to be merged? It would be very unfortunate/annoying if 4.3 came out and suddenly plugins looked ugly. If this is not going to be merged, it would be nice to know as well, because then we can already start programming workarounds.
Thanks.

@akien-mga
Copy link
Member

It needs to be tested to confirm that it solves the issue / is sufficient for the users who were having problems with the previous change.

@Jowan-Spooner
Copy link

Alright I have tested this and it does do the following:

  • readonly_font_color -> font_readonly_color
  • disabled_font_color -> font_disabled_color
  • disabled_highlight_color -> highlight_disabled_color

I would suggest it should also do the following, even thought these are not perfect replacements, but that prevents them from just becoming black:

  • readonly_color (this is one we used very commonly for whatever reason) -> font_readonly_color
  • property_color -> prop_category
  • highlighted_font_color -> font_hover_color

That's all the color-changes/removals I know of.

Alternatively, if left like this, plugin developers will need to change usage of some colors. This PR would then still be an improvement, because we could for some time keep using color names that work in versions before and after 4.3.

@akien-mga
Copy link
Member

@Mickeon Can you add the other suggested compatibility aliases?

If we only handle part of the issue, we're not really solving the problem for plugins aiming to keep using these colors.

@Mickeon Mickeon force-pushed the Some-old-theme-color-renames branch from b6bcbe6 to 5749007 Compare June 6, 2024 11:43
@Mickeon
Copy link
Contributor Author

Mickeon commented Jun 6, 2024

I updated this PR with the above 3 suggestions

@akien-mga akien-mga merged commit dfefdad into godotengine:master Jun 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the Some-old-theme-color-renames branch June 11, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor theme color renames break compatibility for plugins
4 participants