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

Godot LSP reports errors on all .plugged repo files #32

Open
russmatney opened this issue Apr 3, 2024 · 9 comments · May be fixed by #37
Open

Godot LSP reports errors on all .plugged repo files #32

russmatney opened this issue Apr 3, 2024 · 9 comments · May be fixed by #37

Comments

@russmatney
Copy link

Hey there! Thanks for this plugin, it saves a bunch of time while adding and updating external plugins!

Since adding gd-plug, i've noticed that godot lsp happily traverses into the .plugged directory and reports errors from all the cloned addon repos - is there any way to prevent this or some workaround that you're using to avoid seeing all the extra errors for files that aren't used by your game?

Or maybe this is just a problem on my end? I'm using emacs as an external editor and connecting to lsp in the running godot editor instance - maybe it's a problem in my setup.

It feels like godot lsp should ignore files in a folder with .gdignore , but for now this seems like a rare issue - not sure if anyone else is experiencing it.

Thanks for any help or insight!

@imjp94
Copy link
Owner

imjp94 commented Apr 6, 2024

It is the same in vscode =(
Screenshot 2024-04-06 173011
I guess it is because .gdignore only stop importer from importing files, but not native resources like gdscript, see godotengine/godot#20743 (comment)

Solution

I have an idea of support installing plugins with bare repo(see git clones --bare) which would not only resolve the errors but also greatly reduce the footprints of .plugged, since it only pull the git repository itself without a working directory.
But I haven't have the time to test the idea yet...

@HolonProduction
Copy link

As a workaround you could also change the path from res://.plugged to res://addons/.plugged or res://addons/gd-plug/.cache or something like that, as long as it is in the addons directory.

@russmatney
Copy link
Author

russmatney commented Jul 8, 2024

Cool, nice that this feature is coming to godot! In the meantime i've just been deleting res://.plugged completely after every update 🤷

@HolonProduction
Copy link

This PR would not solve this issue. The sentiment from the GDScript team seems to be, that all code outside addons is considered as user code and will produce warnings, so to prevent warnings, the code needs to be in the addons folder (or not checked out in any working directory at all, like suggested before).

The PR only adds a way to enable warnings again for certain addons (which is needed for addon developers).

If you need a quick workaround that isn't deleting the folder: change the path as I suggested before (it's just a const in plug.gd).

@imjp94
Copy link
Owner

imjp94 commented Jul 8, 2024

I just think of an easy way to support this PR in the future.
Change const DEFAULT_PLUG_DIR = "res://.plugged" into environment variable, so that user can change the default path to any path(even outside project directory) without breaking anything.

@Jack-023
Copy link

I just opened #33 which changes the default location for .plugged to /addons/gd-plug which avoids triggering warnings.

@Tattomoosa
Copy link

Tattomoosa commented Sep 10, 2024

Fixed the issue in #33 and opened #37

I did a bit of testing but if someone is interested in putting it through its paces and making sure it handles all cases well, the updated file is here:

https://github.com/imjp94/gd-plug/blob/ead774f6594aac5a7669eb7a2f6b12f716645e0c/addons/gd-plug/plug.gd

@Tattomoosa
Copy link

It seems there's still issues with .plugged even in the addons folder (#37 (comment)). I can't reproduce them reliably but sometimes/for some reason Godot will parse it even though it's hidden, even if .gdignore exists within it. This definitely seems like an engine bug, but unfortunately I haven't been able to create an MVP to open an issue about it.

gd-plug already supported .plugged outside of res:// (try res://../.plugged and it'll work fine) but I had another idea for a workaround where we could keep .plugged in the user:// directory which keeps it outside of where Godot will parse, outside of git, but still tied directly to the project on the end user's machine. This failed due to protection access within DirAccess itself previously, so I submitted a PR that can use another DirAccess to open the user directory

Pull request here: #40
Direct link to plug.gd for testing: https://raw.githubusercontent.com/Tattomoosa/gd-plug/user_dir/addons/gd-plug/plug.gd

@imjp94
Copy link
Owner

imjp94 commented Oct 12, 2024

@Tattomoosa @Jack-023 Thanks for the effort!

I have tested #40 and it works as expected.
I think supporting .plugged in user:// is a really nice feature to add.
But I don't agree with changing default plug directory to user://. It will consume lots of memory without user noticing it, as the project app data folder will always stay in user:// unless manually removed. And there's no easy way to remove it through editor, even clicking the "Remove" button in project list doesn't remove the project folder from user://.
addons/gd-plug would be a more ideal default plug directory.

For the migration, I think what was suggested in #39 is really inspiring.
I think the most elegant way to do it would be adding a config file to store user preferences in addons/gd-plug/gd-plug.cfg, while caching the settings in another file(for example_gd-plug.cfg). Then compare them to detect changes in .plugged directory path, so we don't have to scan the whole project file for both old and new .plugged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants