Skip to content

Fix being able to click through some areas of the chrome#21593

Merged
PunkPun merged 2 commits intoOpenRA:bleedfrom
dragunoff:bugfix/chrome-clickthough
Oct 2, 2024
Merged

Fix being able to click through some areas of the chrome#21593
PunkPun merged 2 commits intoOpenRA:bleedfrom
dragunoff:bugfix/chrome-clickthough

Conversation

@dragunoff
Copy link
Copy Markdown
Contributor

The production palette in RA is assembled from a foreground and a background. The foreground provides most of the visible graphics (such as the metallic chrome) but it has to let clicks go through to the production icons. The background is the one that has to stop the clicks then but it was not wide enough (because the art is only for the background behind production icons and not the whole chrome). Trying to fix that by wrapping the image in wider container that has ClickThrough set to false revealed that there is a bug with the cloning logic for ContainerWidget. It simply did not copy the ClickThrough field and it was always true for cloned widgets. So the value in YAML was lost when the template was cloned.

Before:
image
image
image

After:
image
image
image

…in RA

The production palette in RA is assembled from a foreground and a background. The foreground provides most of the visible graphics (such as the metallic chrome) but it has to let clicks go through to the production icons. The background is the one that has to stop the clicks then but it was not wide enough (because the art is only for the background behind production icons and not the whole chrome). Trying to fix that by wrapping the image in wider container that has `ClickThrough` set to `false` revealed that there is a bug with the cloning logic for `ContainerWidget`. It simply did not copy the `ClickThrough` field and it was always `true` for cloned widgets. So the value in YAML was lost when the template was cloned.
{
ClickThrough = other.ClickThrough;
IgnoreMouseOver = true;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this feels like it's going to produce countless regressions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this was the only place where ClickThrough was used on a ContainerWidget.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To observe this change you need to have set ClickThrough: false and clone it because it's a template for something. That's not going to be a common case.

I could only find a handful of locations with this combo - and it feels like a bugfix for all of them to me.

Background@ICON_TEMPLATE:
Logic: AddFactionSuffixLogic
Width: 66
Height: 50
ClickThrough: false
Background: panel-black

Image@ROW_TEMPLATE:
Width: 226
Height: 48
ClickThrough: false
ImageCollection: sidebar
ImageName: background-iconrow

Image@ROW_TEMPLATE:
Logic: AddFactionSuffixLogic
X: 40
Width: 190
Height: 47
ClickThrough: false
ImageCollection: sidebar
ImageName: background-iconbg

Image@ROW_TEMPLATE:
Logic: AddFactionSuffixLogic
Width: 235
Height: 52
ClickThrough: false
ImageCollection: sidebar
ImageName: background-iconrow

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

ok

@PunkPun PunkPun merged commit 806f0fd into OpenRA:bleed Oct 2, 2024
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Oct 2, 2024

changelog

@dragunoff dragunoff deleted the bugfix/chrome-clickthough branch October 3, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants