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

Improve toggling and clearing debugger breakpoints #101

Open
3 of 6 tasks
NathanLovato opened this issue Jan 10, 2021 · 10 comments
Open
3 of 6 tasks

Improve toggling and clearing debugger breakpoints #101

NathanLovato opened this issue Jan 10, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@NathanLovato
Copy link
Collaborator

NathanLovato commented Jan 10, 2021

We have debugger support since a few months, and it works, but the UX could use some more refinement.

Clearing breakpoints is tedious at the moment at you have to go to every line with a breakpoint and run the remove breakpoint command.

Here are some tasks to improve the UX.

New commands

  • Remove breakpoints in range: loop over all breakpoints in the selected range and remove them.
  • Remove breakpoints in buffer: using the range function above, remove breakpoints in the entire buffer.
  • Remove breakpoints in defun. Same as above but for the defun where the point is, so you don't have to do a selection first. GDScript code is mostly functions (methods) so that could come in handy.

Usability

  • Add shortcut for toggle breakpoint.
  • Remove shortcuts for add and remove breakpoints, they'll be replaced by the above.
  • Add shortcuts that match Godot's debugger and run? F5 to F9 are not bound by default.
@NathanLovato NathanLovato added the enhancement New feature or request label Jan 10, 2021
@NathanLovato
Copy link
Collaborator Author

Improvements started in bdce2da and fe59b77

@xiliuya
Copy link
Collaborator

xiliuya commented Apr 21, 2023

@NathanLovato Is the way godot 4 debugs breakpoints changed?
Run gdscript-godot-run-project-debug .
An error occurred when I added a breakpoint using godot4:

Invalid debug host address, it should be of the form <protocol>://<host/IP>:<port>.

@NathanLovato
Copy link
Collaborator Author

The debugger probably changed in some ways in Godot 4, but I couldn't tell you exactly how.

What I can say is that the plan for Godot is to move to using the debugger adapter protocol (DAP). Currently, it uses its own protocol because the debugger dates back from way before DAP.

Once it gets DAP, all the debugger code in this package can be removed in favor of dap-mode.

@NathanLovato
Copy link
Collaborator Author

Actually, it appears that Godot 4 has DAP support. So all the custom debugger code this package has could be deprecated and replaced with dap-mode support for Godot 4. This might also explain the error you got.

But note that, to my knowledge, this is only Godot 4. Godot 3 still uses its own debugger protocol.

@xiliuya
Copy link
Collaborator

xiliuya commented Apr 21, 2023

@NathanLovato I see, so we need to use another protocol when using godot4, should be implemented in another el file better?

@xiliuya
Copy link
Collaborator

xiliuya commented Apr 21, 2023

Ha ha, I found my mistake.

./Godot_v3.5-stable_x11.64 -h | percol
  --remote-debug <address>         Remote debug (<host/IP>:<port> address).
godot4 -h | percol
  --remote-debug <uri>              Remote debug (<protocol>://<host/IP>[:<port>], e.g. tcp://127.0.0.1:6007).

We can make breakpoints run by modifying the following functions:

(defun gdscript-godot--run-command (&rest arguments)
  "Run a Godot process with ARGUMENTS.

The output of the process will be provided in a buffer named
`*godot - <project-name>*'."
  (let ((args (gdscript-util--flatten arguments)))
    (gdscript-history--add-to-history args)
    (when (and gdscript-debug--breakpoints (not (member "-e" arguments)))
      ;; Start debugger server if it is not running already
      (unless (get-process (gdscript-debug-process-name (gdscript-util--find-project-configuration-file)))
        (gdscript-debug-make-server))
      (push (mapconcat (lambda (breakpoint)
                         (let ((file (gdscript-breakpoint->file breakpoint))
                               (line (gdscript-breakpoint->line breakpoint)))
                           (format "%s:%s" file line))) gdscript-debug--breakpoints ",") args)
      (push "--breakpoints" args)
      
      (if (= gdscript-eglot-version  4)
          (push (format "tcp://127.0.0.1:%s" gdscript-debug-port) args)
        (push (format "127.0.0.1:%s" gdscript-debug-port) args))
      (push "--remote-debug" args))
    (gdscript-comint--run (append (gdscript-godot--build-shell-command) args))
    (setq gdscript-godot--debug-options-hydra :not-list)))

@xiliuya
Copy link
Collaborator

xiliuya commented Apr 21, 2023

Once it gets DAP, all the debugger code in this package can be removed in favor of dap-mode.

So instead of tuning the debugger package for now, should we try adding gdscript support to dap-mode?

@NathanLovato
Copy link
Collaborator Author

Once it gets DAP, all the debugger code in this package can be removed in favor of dap-mode.

So instead of tuning the debugger package for now, should we try adding gdscript support to dap-mode?

For Godot 4 yes, I think so, because dap-mode should provide more features and interactivity than the custom code in this package offers.

@NathanLovato
Copy link
Collaborator Author

@NathanLovato I see, so we need to use another protocol when using godot4, should be implemented in another el file better?

You can, just so that it makes it easy to delete the godot 3-specific file at some point. You can decide to make a stable release of this package dedicated to Godot 3 and at a later date sunset support for Godot 3 in later versions of this repository. Given good documentation, individual users who stick to Godot 3 can always pin their package manager to an older release of this repository.

This is how I would approach this as a maintainer to reduce maintenance cost.

Alternatively, on top of a github release, a Godot 3-specific version of this package could be maintained in a dedicated git branch.

I hope this helps

@xiliuya
Copy link
Collaborator

xiliuya commented Apr 22, 2023

Many thanks for your kind and warm help.

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

No branches or pull requests

2 participants