-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
Rewrite debugger for Godot 4 support + improved maintainability #452
Rewrite debugger for Godot 4 support + improved maintainability #452
Conversation
c5d502c
to
b738e57
Compare
What's the status of this PR? I'm creating a new project on Godot 4 and this would be really helpful to me |
Not done. Due to some other PRs from the queue getting merged this week, it's possible that I'll be able to make progress on this soon ™️, but I'm not making any promises. |
I have both debugger versions living together now. 3 works in both launch and attach mode, but 4 ONLY works in launch mode. I can't get it to connect to a running editor at all. There seem to be some changes in godot's debugger internals, but its difficult for me to tell the extent of them. There are a number of other less severe problems with the scene tree or inspector or cleaning up after a session stops, but those should all be fixable with a little effort. |
Hello I ended up building this PR and testing it with my project in Godot 4, so far it is working correctly, had to make a few minor changes to my settings and launch to adjust to your modifications. I get debug messages now and also I can pause the code. Also seems as though the variables viewer and breakpoints work as intended. I would review the code but I am not well versed in JS it so I am just leaving a comment. |
Thanks, @moonraymurray, that's great to hear. Can I assume you were using the debugger in launch mode, ie |
using
And here is my settings.json, I am using a one or 2 other extensions aswell.
I had to delete references to the other editor paths(default or 3) to get it to run. Sorry about the formatting |
Made some progress on the Godot 3 debugger. I removed the My next task is to doing the same thing to the Godot 4 side, and then hopefully these improvements will help me figure out why Godot 4 attach sessions don't work. I intend to call this PR done once all 4 debugger modes (G3 Launch, G3 Attach, G4 Launch, G4 Attach) are working more-or-less correctly. |
I'm not able to use the debugger (Compiled from you branch) on windows with the following
Using wireshark to debug the communication I observe this error message:
The error comes from vscode sending the project directory using Edit: I just noticed that, while the workaround allows me to launch the game and the external debugger to get attached. Breakpoints are not synced due to similar issue. This is the output I see on wireshark:
|
@Emi1305 That's excellent information, thank you. I'm planning to get some work done on the godot 4 side of things this week, and with any luck I can get this path stuff sorted out. It would be great if you could check back and test it again next weekend, maybe. |
64d6a39
to
f0989fd
Compare
Would love to see this become a reality in the near future |
This is amazing thank you for the work you've done so far on this ! |
Quick status update: I've mostly completely a massive internal refactor to simplify the debugger code. Lots of features still don't work or don't work perfectly, but now it should actually be possible for me to trace out what's happening.
Features I haven't even touched yet:
I want to get sessions (launch/attach, starting, connecting, stopping, resetting) and breakpoints(pause/resume, stepping, stop) working solidly in both versions, and then I can get some volunteer testers and assess the remaining feature list. |
Count me in for testing |
I'd also like to test whenever this is available :) Let me know when testing begins! |
If you want to test this PR's current state, install the VSIX contained in this ZIP archive in VS Code (see instructions): |
I'm unable to launch a debugging session using the provided VSIX. Every attempt to do so results in the following error message:
The part in German says that the executable "godot" was not found. Which is not surprising, because it is not part of my PATH variable. I did set the value for the new Godot 4 Editor setting, however, in the GUI settings editor, it is always reverted to
Additionally, when I try to exit debugging mode by clicking the "Stop" button, the following error message appears and I have to click it a second time to make the debug bar disappear:
|
@DaelonSuzuka |
@Haffi921 this should be it: {
"version": "0.2.0",
"configurations": [
{
"name": "Launch",
"type": "godot4",
"request": "launch",
"editor_path": "godot4",
"project": "${workspaceFolder}"
},
{
"name": "Attach",
"type": "godot4",
"request": "attach"
}
]
} |
Apparently the debugger message format changed between 4.1 and 4.2. It was an easy fix now that I found it. Breakpoints also weren't working for me in the benchmark repo because I added |
Completed a bunch of user-facing polish. Error handling, prompts/dialogs for remediation, logging goes to an output panel (only for the debugger so far). I've removed the commands
Log output screenshot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw is this PR's title still aptly? This looks more like a complete rewrite than just "debugger fixes" :) |
Thanks for taking the time to test this branch, your feedback has been extremely valuable!
We usually fix the PR titles right before merging. |
Regarding this, would it make sense to implement some functionality in Godot for debuggers to request editor icons, like a request using the existing debugging protocol to fetch a particular icon? I think it's a more reliable solution than to bundle the editor's icons like it is currently. If you think this is useful lemme know and I can start a proposal to see what the community's opinion is. |
That's a good idea, but it makes me concerned about the volume of data we'd end up shuffling around. There's already an issue with slowdowns after a breakpoint hits and we have to recursively fetch inspections for all the variables. Also thanks for reminding me that I need to update the icons! |
Possibly offtopic, but this hyped me up so much. Can't wait for the next release! |
This is a continuation of #400 by @RedMser. I'm opening this as a draft PR so I have a place to keep some notes.
Current state:
It may be possible to integrate the changes in #387 into this PR.