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

Avoid saving the texture_rd_rid property of TextureRD resources #87541

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Jan 24, 2024

This fixes an unreported bug I noticed while working on a more advanced demo using Texture2DRDs.

The bug arises when using @tool scripts that modify Texture2DRds. The engine was saving the texture_rd_rid property to disk and then trying to load and initialize the texture using the old RID. That doesn't work because RIDs don't persist between runs.

Also this PR removes the texture_rd_rid from the editor as you can't set the value from editor. It has to be set at run time after allocating an RD texture and obtaining its RID.

CC @BastiaanOlij

@clayjohn clayjohn added this to the 4.3 milestone Jan 24, 2024
@clayjohn clayjohn requested a review from BastiaanOlij January 24, 2024 18:17
@AThousandShips
Copy link
Member

(remember to quote annotations so you don't tag accounts)

RIDs do not persist between runs, so they should not be saved
@clayjohn clayjohn requested a review from a team as a code owner January 24, 2024 18:34
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Makes sense to me, did some related work previously

The editor part also makes perfect sense as they're not repeatable

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

Good find! Looks good to me :)

@YuriSizov YuriSizov merged commit 68d5043 into godotengine:master Jan 25, 2024
16 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@akien-mga akien-mga changed the title Avoid saving the texture_rd_rid property of TextureRD resources Avoid saving the texture_rd_rid property of TextureRD resources Feb 6, 2024
@clayjohn clayjohn deleted the TextureRD-property branch March 13, 2024 21:04
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.

4 participants