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 filled, border_width, editor_only properties to ColorRect #65549

Closed

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Sep 8, 2022

Salvages #42931.
Closes godotengine/godot-proposals#1687.

Ports over the properties of ReferenceRect to ColorRect itself.

Showcase

This also renders ReferenceRect impressively redundant, but... It'd be very last minute to... ✂️ ✂️ ✂️ #65559

@Mickeon Mickeon marked this pull request as ready for review September 8, 2022 22:51
@Mickeon Mickeon requested review from a team as code owners September 8, 2022 22:51
@YeldhamDev YeldhamDev added this to the 4.0 milestone Sep 8, 2022
Ports over the properties of ReferenceRect to ColorRect itself. Also adds documentation.
@seppoday
Copy link

seppoday commented Sep 9, 2022

This might be not good place for such comments but it might be nice to also have border color? It would be possible to do color fill with border with different color. For example black box with red border.

@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 9, 2022

Having both fill and border color? That's not necessarily a bad idea, but by that point, StyleBoxFlat exists too for the same purpose, and has more features, too...

I suppose it's at least worth considering, if this and #65559 are approved.

} break;
}
}

void ColorRect::_validate_property(PropertyInfo &property) const {
Copy link
Member

Choose a reason for hiding this comment

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

Should be p_property.

@akien-mga
Copy link
Member

We discussed this in a PR review meeting and decided to keep the current set of nodes simple:

  • ColorRect for colored rects
  • ReferenceRect for rects with a border that can be editor-only (debugging)
  • StyleBoxFlat for any more advanced use case

Merging ReferenceRect features into ColorRect and removing ReferenceRect as suggested here would also be a viable option, but it also means that ColorRect gets even more overlap with StyleBoxFlat features and users might well ask for more features like rounded corners, etc., so we end up duplicating logic which is already supported in StyleBoxFlat.

@Calinou suggested opening a proposal to allow converting a ColorRect to a Panel + StyleBoxFlat with the chosen color, which can be a base for more advanced use cases.

@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 20, 2022

Not the place for discussion, but for the same goals of Calinou's proposed solution, I feel like it would also be a good idea to make these types of Resources easier to set up and manage, like, in a way, ColorRect already is, simple as it is.

No concrete proposed solution, just an afterthought.

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.

Expose filled, width and antialiased properties in the ColorRect class
4 participants