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

Make project setting export respect to convert_text_resources_to_binary #90999

Closed
wants to merge 1 commit into from

Conversation

jsjtxietian
Copy link
Contributor

Fixes #90981

Tested locally it seems all is ok since we use _load_settings_text_or_binary("res://project.godot", "res://project.binary") to load the setting all the time.

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.

Glanced at the code for the project settings and it doesn't seem like it expects binary explicitly in exports, I'd argue the original issue comes from invalid documentation references rather than faulty code, but dont think it hurts to respect this setting here, though I also don't necessarily think there's any reason to not store it as binary

But for my part I'd say to go with this

A second pair of eyes from someone would be good though on the relevant area

@jsjtxietian
Copy link
Contributor Author

what a nice pr id, 90999

@akien-mga
Copy link
Member

Discussed briefly with @reduz, it's not clear whether we should actually make this change. Project Settings aren't a resource so technically they're not related to that setting.

We need to assess what's the use case for exporting the project.godot file as is, and whether this setting (or a new one) is the right way to enable it. If the problem is only that the documentation was misleading, we can fix the documentation instead.

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