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

Highlight hovered GraphEdit connection by widening the line #100456

Merged

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Dec 16, 2024

Currently, GraphEdit connections are highlighted by tinting the color. With this change, the connections are highlighted by widening the line by a configurable factor.

The current method of highlighting connections has the following two properties:

  1. The tint color is global for all connections, so when different colors are used, some of them can be very similar to the tint color, so that there is nearly no visual feedback for some highlighted connections.
  2. If the tint color is darker than the connection color, the connection will be displayed darker while hovering. So while hovered It takes a back seat visually, even though the hovering should actually emphasize the connection.

Replacing the tint color by a configurable theme option solves these issues and also additionally allows users to change the settings in a way, that ho highlight happens.

While this PR changes the theme-API and is therefore technically compatibility breaking, the GraphEdit node is still marked as experimental, so this should not be a deal-breaker. This is no longer an issue, since theme properties are no longer removed.

CC @Geometror

Updated 2024-12-18: Set default values to 0%.

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I like this idea, and it fits pretty good in the VisualShader editor, I'm against removing the tint color.
If someone doesn't like highlighting via color, they can just set the tint color to a color with an alpha value of 0.
By setting the default value of connection_hover_thickness to 0%, we wouldn't even need to break compatibility.

@Sauermann Sauermann force-pushed the proposal-hovered-connection-highlight branch from 09a5527 to 5a183ee Compare December 16, 2024 22:04
@Sauermann Sauermann force-pushed the proposal-hovered-connection-highlight branch from 5a183ee to 53944bf Compare December 16, 2024 22:11
@Sauermann
Copy link
Contributor Author

Sounds good. I have removed the removal of the tint color.

@Geometror
Copy link
Member

Even if GraphEdit is experimental, I'd suggest using 0% as a default value for the widening, as we would still kind of break compatibility (in the manner of a changed default visual appearance that some users might not prefer).
Adjusting the editor theme should be done in a subsequent PR then.

@Sauermann Sauermann force-pushed the proposal-hovered-connection-highlight branch from 53944bf to 04077da Compare December 18, 2024 22:27
This change causes the connections to be additionally highlighted by
widening the line with a configurable factor.
@Sauermann Sauermann force-pushed the proposal-hovered-connection-highlight branch from 04077da to 0a875ab Compare December 19, 2024 23:56
@Repiteo Repiteo modified the milestones: 4.x, 4.4 Dec 20, 2024
@Repiteo Repiteo merged commit 075567a into godotengine:master Dec 20, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 20, 2024

Thanks!

@Sauermann Sauermann deleted the proposal-hovered-connection-highlight branch December 20, 2024 02:15
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.

3 participants