godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.47k stars 148 forks source link

Invalid Executable with latest 4.3 Beta #585

Closed chrisl8 closed 4 months ago

chrisl8 commented 5 months ago

Godot version

Godot Engine v4.3.dev.custom_build.d3352813e

VS Code version

1.86.0

Godot Tools VS Code extension version

Latest from master branch

System information

Windows 11

Issue description

I think that this Godot Engine commit: https://github.com/godotengine/godot/commit/f7f51bdd7a5b73143ef126c85f767cb5d5b54e84

may be breaking the pattern matching code found here: https://github.com/godotengine/godot-vscode-plugin/blob/e2f2dc4b93eec68ebdadd76255746b8b0c0ca9d7/src/utils/project_utils.ts#L84

The match appears to fail every time now that the Godot help output is in color.

I know this isn't even a dev release, but I'm throwing this out here now so we can get ahead of it.

I will update this issue if the next Dev snapshot does or does not also break it. I assume this new code will stay though.

Steps to reproduce

Build the latest version of Godot Engine from the master branch and use it with this extension.

chrisl8 commented 5 months ago

Changing the line to this appears to fix the problem:

const pattern = /^.*Godot Engine.*v(([34])\.([0-9]+)(?:\.[0-9]+)?)/;

This is such a trivial change I hesitate to make a PR, but I can if you want.

Also, it might be worth holding off until 4.3 is at least in Beta or Dev and confirm that the new output is final.

DaelonSuzuka commented 5 months ago

Hey, thanks for catching this!

Your proposal works, but my gut says it would be better to strip the non-printable[1] characters from the string first and then use the existing regex. .* is a lot of freedom that I don't necessarily want to allow.

Also, it might be worth holding off until 4.3 is at least in Beta or Dev and confirm that the new output is final.

I agree, let's wait a bit and see if they add anything else.

[1] terminal colors and other control functions are done using https://en.wikipedia.org/wiki/ANSI_escape_code, which are (more or less) ASCII characters that do not have printable representations.

chrisl8 commented 5 months ago

Your proposal works, but my gut says it would be better to strip the non-printable[1] characters from the string first and then use the existing regex. .* is a lot of freedom that I don't necessarily want to allow.

Also, it might be worth holding off until 4.3 is at least in Beta or Dev and confirm that the new output is final.

I agree, let's wait a bit and see if they add anything else.

[1] terminal colors and other control functions are done using https://en.wikipedia.org/wiki/ANSI_escape_code, which are (more or less) ASCII characters that do not have printable representations.

Yep, totally makes sense. I was just using the simplest fastest approach. Since this is only here as a mild help to the user, I figured if it accidentally flags something as "OK" when it isn't wouldn't be the end of the world. That and until it goes to at least a Dev release or maybe Beta, it could change.

As discussed a bit over here https://github.com/godotengine/godot-vscode-plugin/pull/586#issuecomment-1930666291 the whole thing is a bit weird anyway. In theory one would use something like --version but that isn't super helpful either.

It would be nice if the Godot Engine had a documented API for finding the full version string, but that may be asking too much right now.

If I happen to come up with a fancier version I'll make a PR, but don't wait on me.

DaelonSuzuka commented 4 months ago

@chrisl8 We decided to just go back to using --version, which doesn't have colors and thus, doesn't have these problems.