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.

Sign up for GitHub

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

Jump to bottom

C#: Fix command line exporting #79173

Merged
merged 1 commit into from
Jul 8, 2023
Merged

C#: Fix command line exporting #79173

merged 1 commit into from
Jul 8, 2023

Conversation

RedworkDE
Copy link
Member

During the export reload_registered_script is called a few times with scripts that have no path assigned and then when update_file is called with an empty string, things break.
I didn't track down exactly what happens and just added an error to prevent future problems like this (I can remove it again if someone thinks there is a valid use for it tho)

@RedworkDE RedworkDE added bug topic:dotnet topic:export cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 7, 2023
@RedworkDE RedworkDE added this to the 4.2 milestone Jul 7, 2023
@RedworkDE RedworkDE requested a review from a team as a code owner July 7, 2023 18:57
@raulsntos raulsntos mentioned this pull request Jul 7, 2023
Closed
raulsntos
raulsntos approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

LGTM. The maintainers of EditorFileSystem should probably review this too, to make sure the added error is fine.

@raulsntos raulsntos requested a review from a team July 7, 2023 19:14
YuriSizov
YuriSizov approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@YuriSizov YuriSizov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The error is kind of redundant in the sense that the method should never be called with an empty value, and you ensure that with the second change. But I guess it doesn't hurt to have a sanity check.

@akien-mga akien-mga merged commit 157973a into godotengine:master Jul 8, 2023
@RedworkDE RedworkDE deleted the net-commandline-exporting branch July 8, 2023 16:25
@akien-mga
Copy link
Member

Thanks!

@DillonSteyl
Copy link

Super impressed with how quickly this was handled. Amazing work

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

Cherry-picked for 4.1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuriSizov YuriSizov YuriSizov approved these changes

@raulsntos raulsntos raulsntos approved these changes

Assignees
No one assigned
Labels
bug regression topic:dotnet topic:export
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

4.1 Mono Regression - Export via CLI is broken (not saving all required resources)
5 participants
@RedworkDE @akien-mga @DillonSteyl @YuriSizov @raulsntos