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

Enable Deploy with Remote Debug by default in the editor #99428

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Nov 19, 2024

This makes one-click deploy make use of remote debugging, so that you can see output from the remote device, run the debugger and use the monitor and performance/network profilers.

This makes one-click deploy make use of remote debugging, so that
you can see output from the remote device, run the debugger and use
the monitor and performance/network profilers.
Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that 99% of the time, when running a project, you want debugging info.

@mhilbrunner
Copy link
Member

mhilbrunner commented Nov 20, 2024

I've run into this recently when working on other platforms and was likewise surprised.

My guess is it is off by default because enabling it makes Godot open a network reachable port? So it has implications:

  • if on by default, Godot by default triggers network security / firewall
  • it would mean running Godot always exposes the Godot executable as attack surface on the local network, which means any security issue in/around debugging/networking/serialization etc. now become "you can by default own any Godot started on the local network", which may be a company wide network in case of a game dev studio with hundreds of seats
  • it may elevate the chance of false positives of antivirus heuristics (hopefully, not much, because malware opening ports isn't really the state of the art anymore)

IMO, enabling it by default is what I'd expect, and personally, I'd make that tradeoff. However, I see the cons, and I think we currently make whether to connect to the internet (Asset Lib, update check etc.) a one-time prompt? If I'm not misremembering and that prompt does exist, we could make this part of it.

I agree that 99% of the time, when running a project, you want debugging info.

I'd hazard a guess that 99% of the time, users are not running remote deployments. As we lack analytics, I have no idea what our mobile user share is, but I'd expect only mobile dev (and other platforms with dedicated hardware, like consoles) to use that. And even then, typically, you only actually test on a real device every so often, as iterating on the dev machine is faster.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 20, 2024

Note that this only affects one-click deploy, i.e. running on another device.
Also network security / firewall is already triggered when you run Godot, because the debugger works only over network.

@mhilbrunner
Copy link
Member

mhilbrunner commented Nov 20, 2024

Also network security / firewall is already triggered when you run Godot, because the debugger works only over network.

There is a difference between opening a localhost bound port by default and opening a network-reachable port by default, but IIRC you're still right that at least the Windows scare screen may be dumb sensitive enough to not differentiate.

Edit: Actually, according to a brief search, binding to 127.0.0.1 should not trigger the firewall, apparently. So if we do trigger it by default, thats curious and worth a look, IMO.

@akien-mga akien-mga requested a review from Faless November 29, 2024 16:33
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say let's give it a try now for dev7, and see what happens?

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Dec 17, 2024
@Repiteo Repiteo merged commit 76d66d4 into godotengine:master Dec 20, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 20, 2024

Thanks!

@Calinou Calinou deleted the editor-default-enable-remote-deploy-debug branch December 21, 2024 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Deploy with Remote Debug by default in the editor
6 participants