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

Fix ShaderGlobalsOverride property handling #82100

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

bitsawer
Copy link
Member

@bitsawer bitsawer commented Sep 22, 2023

A simple typo in property name remapping stopped the overrides from working properly. The properties were actually serialized correctly, but because the remapped names came out wrong they didn't show correct values in the inspector or have any visual effect in rendering after a reload.

@bitsawer bitsawer added this to the 4.2 milestone Sep 22, 2023
@bitsawer bitsawer requested a review from a team as a code owner September 22, 2023 07:31
@rstecca
Copy link

rstecca commented Sep 22, 2023

Good job spotting this. I dare say variable names could be better here.

@OhiraKyou
Copy link

OhiraKyou commented Sep 24, 2023

I dare say variable names could be better here.

Yeah, I was thinking the same thing. Outside of very short and simple math wrappers, where a and b represent operands of the same type whose order doesn't matter (commutative operations), I generally prefer to strictly avoid single-letter variable names—even for loop indices.

@bitsawer bitsawer requested a review from a team October 11, 2023 12:00
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good.

@clayjohn clayjohn added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 11, 2023
@akien-mga akien-mga merged commit 9e02dcd into godotengine:master Oct 11, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

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.

ShaderGlobalsOverride - Params changes lost on reload
6 participants