-
Notifications
You must be signed in to change notification settings - Fork 26
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 Input Action and NodePath completion #102
Conversation
Closes #86 |
NodePath completion should work now exactly how it works in VS Code and Godot Editor. It queries against the currently open scene and accounts for relative script location in the scene tree. |
Thank you! I will take a look shortly. |
What are options for adding automated tests for this completion? |
f4afd5e
to
ba68979
Compare
I was thinking about that as well. And you are correct the Godot Editor has to run in order to accept connections from Rider. We could mock the Editor part but that would only accomplish testing the mock implementation imho. The only part that would be interesting to test would be that the correct methods are called in |
ba68979
to
89314bc
Compare
I would be happy even without those tests for now. I just wanted to clarify. |
...ns/ResCompletionTest/.godot/editor/NewScript.cs-folding-c4b8f5e741d04006ec5d8d0c099949b5.cfg
Outdated
Show resolved
Hide resolved
also avoid creation GodotMessagingClient for non-Godot projects
76b7fca
to
99a6aab
Compare
8cb1400
to
240e4c5
Compare
Wow, this is so cool |
@jsbeckr Is there anything else, which you want to accomplish within this PR? |
Nope! I am super happy with this! 👍 |
👍 |
@jsbeckr some of your commits were from a different git user, I have fixed that and force-pushed. |
Plugin is published - 2023.2.121. |
Great job guys, I love this! Testing this with the master version of godot (rider EAP 6) has mixed result: Sometines, When I open a C# script from Godot, it seems that rider can't connect to godot and I get a null reference when asking for a completion (cannot reproduce right now, I will send the stacktrace when I get a new one). I could not find the icon indicating the status of the connection to Godot... This error goes away if I restart rider. Input completions work great 👍 However, for NodePath completions I get errors in the Godot console:
This is probably linked to the new Threading model in godot 4.1 (see godot#78149) Did any of you test this with the 4.1 RC version of Godot? |
I was using Godot 4.0.3 |
The icon is in the top right next to the build and run icon. Maybe it's only visible with Next UI enabled? The exception that you see should be an internal problem of Godot. We just connect via TCP and send a message. I'm not not at home right now so it's hard to test for me. Would be a shame if it's not working for 4.1. |
Yes I see the icon, I was expecting it to be on the bottom panel for some reason 😄 Might be a godot bug, the new threading model has been causing a few issues here and there, maybe the interaction with the IDE messaging has been undetected before. |
The connection indicator should work in both new/old UI. I have tried that. |
I guess we have to open a bug for that. Indeed NodePath completion does not work in 4.1 right now. Curiously enough it's not Godot (I think). During development of this feature I build the plugin to use it in my current Rider 2023.1.3 installation. With that version I can still get NodePath completion. So in my opinion we introduced the problem during the refactoring (but that only affects 4.1 for whatever reason) or it has something to do with the Rider EAP 2023.2 in combination with Godot 4.1. Which also sounds weird. ¯_(ツ)_/¯ |
I implemented input action and
NodePath
completion by using https://github.com/godotengine/godot/tree/master/modules/mono/editor/GodotTools/GodotTools.IdeMessaging (it's MIT). I had to copy the code from the Godot repo, because the nuget version is outdated. But that would be just a temporary thing, because Godot plans to publish proper nuget packages for communicating with the Godot IDE in the future. Once that's done we would integrate them and remove theGodotTools.IdeMessaging
code and assembly from the plugin.TODO
Possible Improvements:
ResourcePath
completion to use theIdeMessaging
as wellScenePaths
completionPlayRequest
s /OpenFileRequest
s viaIdeMessaging
NodePath Completion
Screen.Recording.2023-06-25.at.19.36.59.mov
Input Action Completion
Screen.Recording.2023-06-25.at.19.37.34.mov
P.S.: I signed the contributor agreement.