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

Add --import command-line flag #90431

Merged
merged 1 commit into from Apr 9, 2024
Merged

Conversation

mihe
Copy link
Contributor

@mihe mihe commented Apr 9, 2024

Closes godotengine/godot-proposals#1362.
Supersedes #68461.

This adds a new command-line option called --import, the purpose if which is to launch the editor (headless or not) solely to import any non-imported resources and then quit, meaning it implies --editor and --quit.

This also changes --export-release, --export-debug and --export-pack to imply --import.

This should hopefully remove the need for hacks such as timeout 25s godot --editor or godot --editor --quit-after 1000.

This is fairly similar to #68461 in that it uses EditorFileSystem::first_scan as the condition variable by which to wait for, which seems to more or less guarantee that the scan thread has had time to finish and that any imports have gotten queued up and processed.

This PR differs from #68461 in that it does not imply --headless. As talked about in that PR there may still potentially be issues with exporting from headless (although I haven't seen any showstoppers in my limited testing), so I figured it's best to leave that choice up to the user. Any such issues should in my opinion be considered separate anyway, and not block this seemingly straight-forward and useful addition.

(I'll create a corresponding godotengine/godot-docs PR shortly.)

@mihe mihe marked this pull request as ready for review April 9, 2024 13:12
@mihe mihe requested a review from a team as a code owner April 9, 2024 13:12
@AThousandShips AThousandShips added this to the 4.x milestone Apr 9, 2024
main/main.cpp Outdated Show resolved Hide resolved
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.

Looks very straightforward. That's in line with the kind of implementation I've often suggested over the years.

I thought it would require a bit more changes on the EditorFileSystem side but reusing first_scan seems like a good option. The EFS code around it seems a bit convoluted in some parts, so I wonder if this will fully cover all use cases when doing e.g. multithreaded imported of some resource types. But testing should confirm that.

@mihe
Copy link
Contributor Author

mihe commented Apr 9, 2024

It would probably be a good idea to also allow this --import to make use of the existing command-line logging that --export-* currently enables, aka EditorNode::cmdline_export_mode, but the way that's currently enabled is somewhat questionable, where it disables any progress popup even in non-headless mode, so I'd like to refactor it a tiny bit in the process, which seemed better suited to a separate PR.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 9, 2024
@akien-mga
Copy link
Member

Tested successfully on Linux with a few demos, all without .godot folder / from a clean Git checkout:

  • https://github.com/godotengine/tps-demo/ with godot --import -> I can then play the project directly with godot
  • W4 Games' Nuku Warriors (not public yet) with godot --import --headless -> Likewise, it imports everything successfully to play the game right away
  • https://github.com/gdquest-demos/godot-4-3d-third-person-controller with godot --headless --export-debug "Linux" ../test-export/tps.x86_64 -> The implicit --import works fine, and the game is successfully exported and playable (note I had to add the export_presets.cfg manually after git clean -fxd).

@mihe
Copy link
Contributor Author

mihe commented Apr 9, 2024

I've been testing with godotengine/tps-demo and gdquest-demos/godot-4-3d-third-person-controller on Windows myself, although I had to add some sleeping to EditorFileSystem::_scan_new_dir in order to reproduce the failure case, as the scanning seems to be much quicker on Windows for some reason.

Alternatively you can on Windows also run it headless through WSL to reproduce the issue without any sleeping.

@fire
Copy link
Member

fire commented Apr 9, 2024

I like the design of --import, but at the moment haven't ran the new command flag.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

Approving the feature.

@akien-mga akien-mga merged commit 0993df8 into godotengine:master Apr 9, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! 🎉

@Calinou
Copy link
Member

Calinou commented Apr 9, 2024

For future reference, here's a script to test all demo projects one by one (run this at the repository clone root):

GODOT_BINARY=path/to/godot.binary

git clean -dfxi

for demo in **/*.godot; do (echo $(dirname $demo) && cd $(dirname $demo) && "$GODOT_BINARY" --import && "$GODOT_BINARY" --quit-after 100); done

I've tested all demos on Linux and they all work as expected with --import.

rsubtil pushed a commit to retrohub-org/godot that referenced this pull request Apr 10, 2024
Add `--import` command-line flag
@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@mihe
Copy link
Contributor Author

mihe commented Apr 18, 2024

Here are the cmdline_export_mode changes I mentioned: #90806

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.

Add option to import resources using a command line argument
5 participants