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

VariantParser make readahead optional #69961

Merged

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Dec 12, 2022

It turns out some areas are independently moving / reading filepointers outside of the VariantParser, which can cause the readahead caching to get out of sync.

This PR makes the VariantParser readahead to be optional to allow for these use cases.

Fixes #69794 (as far as I can see so far)
Alternative to #69835 (until we have time to finalize that PR)

Notes

  • Ideally we would like to use readahead wherever possible, but some cases are less straightforward, so being able to simply turn it off is useful.
  • With more time available I'm sure we can get the readahead working with ResourceLoaderText, but this PR is to fix the regression as simply as possible, and allow us to work at leisure on improving ResourceLoaderText performance.

It turns out some areas are independently moving / reading filepointers outside of the VariantParser, which can cause the readahead caching to get out of sync.

This PR makes the VariantParser readahead to be optional to allow for these use cases.
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Tested the 3.x version to confirm that it fixes the bug, I assume this one does too.

@lawnjelly
Copy link
Member Author

Tested the 3.x version to confirm that it fixes the bug, I assume this one does too.

Worked for me when I tested it in master.

@akien-mga akien-mga merged commit ba4bd7f into godotengine:master Dec 12, 2022
@akien-mga
Copy link
Member

Thanks!

@lawnjelly lawnjelly deleted the variant_parser_optional_readahead branch December 12, 2022 18:09
@TokisanGames
Copy link
Contributor

This also works for me in gd4. Thanks for the fix.

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.

Moving/renaming resources cause unopened/unused tscn/res files to become corrupted
3 participants