search

godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.55k stars 163 forks source link

Expose Godot path as getGodotPath command #586

Closed bitwes closed 7 months ago

bitwes commented 8 months ago

Overview

This PR adds a godotTools.getGodotPath command so that other extensions that rely on this one can get the path to the executable without having to determine the version of the current project and then decide which configuration value to use (godotTools.editorPath.godot3/4).

Usage: In another VSCode extension use the following to get the path to the Godot executable for the currently opened project

let editorPath : string = await vscode.commands.executeCommand('godotTools.getGodotPath');

Justification

I maintain the gut-extension plugin that allows you to run GUT unit tests through VSCode. It was using godot_tools.editor_path to launch Godot. I was starting to adjust my code to determine which configuration value to use when I realized that this extension should probably provide that functionality. This makes it available to other extensions. This also means that if the logic to get the path changes (such as when Godot 3 becomes unsupported) other extensions won't have to be updated.

Questions:

  1. Should I create an issue for this first?
  2. I wasn't 100% sure what .replace(/^"/, "").replace(/"$/, ""); was accomplishing so I left it as a comment inside the new get_godot_path function. Let me know if it should be included or not and I will update the PR.
  3. I had to update the regex pattern used in verify_godot_version to get this to run using version 3.5.3 on a Mac/zsh. I added m to the end so it would find the version regardless of which line it appears on. The -h flag is spitting out the arguments passed to Godot first, so the "Godot Engine" line was not being found. Let me know if I should remove this and make another PR and/or Issue.

Here's the start of my Godot 3.5.3 -h Output:

arguments
0: /Applications/Godot_3.5.3.app/Contents/MacOS/Godot
1: -h
Current path: /Users/butchwesley/development/godot/guts/gut-extension/GodotTestProject/Godot3
Godot Engine v3.5.3.stable.official.6c814135b - https://godotengine.org
Free and open source software under the terms of the MIT license.
(c) 2014-present Godot Engine contributors.
(c) 2007-2014 Juan Linietsky, Ariel Manzur.

...
DaelonSuzuka commented 8 months ago

I sent you a discord message back in November about this exact issue but I never heard from you! Is there a better way to reach you?

Answers

Should I create an issue for this first?

Nope, this PR is perfect.

I wasn't 100% sure what .replace(/^"/, "").replace(/"$/, ""); was accomplishing so I left it as a comment inside the new get_godot_path function. Let me know if it should be included or not and I will update the PR.

This is removing surrounding quotes to normalize the path in the case the user tries to "help" by adding their own quotes.

I had to update the regex pattern used in verify_godot_version to get this to run using version 3.5.3 on a Mac/zsh. I added m to the end so it would find the version regardless of which line it appears on. The -h flag is spitting out the arguments passed to Godot first, so the "Godot Engine" line was not being found. Let me know if I should remove this and make another PR and/or Issue.

I was totally confused reading this because I forgot that I stopped using --version because its output wasn't distinctive enough. Me: "-h? What idiot would use -h!? \ ...oh."

Where are the args coming from? zsh? I'm using Godot 3.5.3 on Win10/cmd and I don't get the args.

Adding the multiline flag should be fine, I'm just curious about the behavior.

My Questions

  1. Are there any other functions that it might be useful to expose? getProjectVersion or anything else?
  2. What do you want getGodotPath to return if there's no project found? Those project utisl are kinda sloppy on error handling because they're for internal use, but an external API is a bit more serious. I also haven't put any effort into supporting multi-root workspaces or other weird situations..
bitwes commented 8 months ago

I was totally confused reading this because I forgot that I stopped using --version

I also thought version might be a better choice, but I ran it and saw that it might be harder to pick out the line with the version on. I'm not sure where the argument output is coming from. I just double checked and it shows up for me when using --version as well.

➜  ~ /Applications/Godot_3.5.3.app/Contents/MacOS/Godot --version
arguments
0: /Applications/Godot_3.5.3.app/Contents/MacOS/Godot
1: --version
Current path: /Users/butchwesley
3.5.3.stable.official.6c814135b
➜  ~ /Applications/Godot_3.5.3.app/Contents/MacOS/Godot --help
arguments
0: /Applications/Godot_3.5.3.app/Contents/MacOS/Godot
1: --help
Current path: /Users/butchwesley
Godot Engine v3.5.3.stable.official.6c814135b - https://godotengine.org
Free and open source software under the terms of the MIT license.
(c) 2014-present Godot Engine contributors.
(c) 2007-2014 Juan Linietsky, Ariel Manzur.

Usage: /Applications/Godot_3.5.3.app/Contents/MacOS/Godot [options] [path to scene or 'project.godot' file]

It does not show up in 4.2

➜  ~ /Applications/Godot_4.2.app/Contents/MacOS/Godot --version
4.2.stable.official.46dc27791
➜  ~ /Applications/Godot_4.2.app/Contents/MacOS/Godot -h
Godot Engine v4.2.stable.official.46dc27791 - https://godotengine.org
Free and open source software under the terms of the MIT license.
(c) 2014-present Godot Engine contributors.
(c) 2007-2014 Juan Linietsky, Ariel Manzur.

Are there any other functions that it might be useful to expose? getProjectVersion or anything else?

I was tempted to expose more things, and almost added the project version as well. Then I figured it was probably best to just wait and see if anyone asks for other things first. Getting the path hides a couple of behaviors of the extension that nobody should have to worry about from the outside. If someone wants the project version, it's in a config file which they should be able to read easy enough.

What do you want getGodotPath to return if there's no project found?

I don't know, undefined? I felt like get_godot_path was a bit too loose with what it was doing, but my TypeScript isn't the best, and I figured code review process would tidy it up. I'm good with any checks or return values you think should be added.

I was thinking that anyone using my tool was already using this one. So if it wasn't configured correctly then they would have figured that out before trying to use mine. That's probably not safe to assume though.

DaelonSuzuka commented 7 months ago

Thanks @bitwes. Feel free to let us know if there's anything we can do to support your extension moving forwards.