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 incorrect check on importing project #77832

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Jun 4, 2023

Note: #77760 makes this impossible to test on some platforms, see there, reverting the cause helps verify this fix

(This problem also occurs on 3.x, have not tested if this can be cherry-picked)

Copy link
Member

@RedworkDE RedworkDE left a comment

Choose a reason for hiding this comment

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

Tho it's more improve than fix, if the folder name ends with .zip it still breaks in the same way, and also selecting a project to import by typing into the text field instead of using the browse dialog seems pretty broken in general

@AThousandShips
Copy link
Member Author

AThousandShips commented Jun 4, 2023

That's true, should add a check for folders perhaps

It is indeed pretty broken, it does not update correctly to do install instead of import, think the fix here is sufficient on its own, though there are absolutely reasonable and valid cases where folders end in .zip

Will look at doing some wider fixes to the updating of the path, if I end up having the time and energy to do so soon I'll add it to this as well

@AThousandShips
Copy link
Member Author

Got some ideas digging into some of the code, will test them later today probably, marking as draft until then

@AThousandShips AThousandShips marked this pull request as draft June 4, 2023 11:53
@AThousandShips
Copy link
Member Author

Some improvements, it's still pretty inefficient and spaghetti, and if you point to a project.godot file it doesn't work still, but now it manages folders ending in .zip, and makes the install path appear and vanish when you change the import path

This needs a major cleaning but at least now it should work correctly, will clean up some of the code though

@AThousandShips AThousandShips force-pushed the import_fix branch 2 times, most recently from d31d38e to a137694 Compare June 4, 2023 13:38
Copy link
Contributor

@idbrii idbrii left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

editor/project_manager.cpp Outdated Show resolved Hide resolved
editor/project_manager.cpp Outdated Show resolved Hide resolved
bool is_zip = false;
if (d->change_dir(base_path) == OK) {
valid_path = base_path;
} else if (is_zip_file(d, base_path)) {
Copy link
Member Author

@AThousandShips AThousandShips Jun 5, 2023

Choose a reason for hiding this comment

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

Reordering here in the weird case where there's a file matching but not a directory, unsure if this will ever happen, but it can't hurt, though I'm not even sure trailing and leading spaces are valid for paths in any OS

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 8, 2024
@akien-mga akien-mga merged commit 049da90 into godotengine:master Jan 8, 2024
15 checks passed
@AThousandShips AThousandShips deleted the import_fix branch January 8, 2024 11:11
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

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.

"This directory already contains a Godot project" when inputting a valid path without trailing /
4 participants