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

TextureProgressBar minimal size is prioritized to under -> over -> progress textures #100693

Open
arkology opened this issue Dec 21, 2024 · 0 comments

Comments

@arkology
Copy link
Contributor

Tested versions

Reproducible since Godot was opensourced.

System information

Not specific to hardware.

Issue description

As title says, TextureProgressBar minimal size is prioritized to under -> over -> progress textures:

Size2 TextureProgressBar::get_minimum_size() const {
	if (nine_patch_stretch) {
		return Size2(stretch_margin[SIDE_LEFT] + stretch_margin[SIDE_RIGHT], stretch_margin[SIDE_TOP] + stretch_margin[SIDE_BOTTOM]);
	} else if (under.is_valid()) {
		return under->get_size();
	} else if (over.is_valid()) {
		return over->get_size();
	} else if (progress.is_valid()) {
		return progress->get_size();
	}

	return Size2(1, 1);
}

So, if someone uses textures with different sizes, they will be surprised that size is not selected as maximum among all "layers".
For example, this is how it looks inside GridContainer:
Image

And this is how it will look if we change minimum size calculation from proprity-based to maximum among all "layers":
Image

To fix it I changed function to the following:

Size2 TextureProgressBar::get_minimum_size() const {
	if (nine_patch_stretch) {
		return Size2(stretch_margin[SIDE_LEFT] + stretch_margin[SIDE_RIGHT], stretch_margin[SIDE_TOP] + stretch_margin[SIDE_BOTTOM]);
	} else {
		Size2 size;
		if (under.is_valid()) {
			size = under->get_size();
		}

		if (progress.is_valid() && progress->get_size() > size) {
			size = progress->get_size();
		}

		if (over.is_valid() && over->get_size() > size) {
			size = over->get_size();
		}
		return size;
	}

	return Size2(1, 1);
}

Of course, it could be solved by enabling nine-patch stretching with zero margins and setting a custom minimum size. And, of course, if no one even reported it before, perhaps no one needs this change.

BUT if changing from priority-based approach to maximum is welcomed, I'll open PR to fix it. I'm sure that "maximum among all layers" is proper behaviour.

Steps to reproduce

Create TextureProgressBar and assign to it textures with different sizes.

Minimal reproduction project (MRP)

New Game Project.zip
Actually this one is my playground to test different TextureProgressBar issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants