godotengine / emacs-gdscript-mode

An Emacs package to get GDScript support and syntax highlighting.
GNU General Public License v3.0
306 stars 33 forks source link

Improve toggling and clearing debugger breakpoints #101

Open NathanLovato opened 3 years ago

NathanLovato commented 3 years ago

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

Usability

NathanLovato commented 3 years ago

Improvements started in bdce2da7941d3e7a60b9fcd49770716fd2b1c3fb and fe59b77c3fa8090d0f322a4dc5b7cad10c59aeb7

xiliuya commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

xiliuya commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

Many thanks for your kind and warm help.