Automatic remote debugger port assignment

[zaksnet]

This is an attempt to automatically assign a free port for the remote debugger when you want to run multiple instances of the editor. This should be especially helpful when someone wants to test a network server/client situation and solves the problem of needing multiple editor files in order to have different settings. In fact, in Linux(i suppose in windows also) i can not run the same version of the editor and have different settings.
A few points for review:

• The code changes the Remote debug port settings of the editor. Although i dont particularly like overriding the settings, this seems to revert back to what it was after the editor is restarted.

Added a warning for when the port is changed:

NOTE: If a user opens and then closes the editor settings after the port has been changed then the new port settings will be saved.
EDIT: This does not seem to be working in windows. server->listen(remote_port) seems to be returning OK even if the port is used by another program. Fixed by #36321

[Faless]

I don't understand this.... can you elaborate further?
Why the delay in the while true?
How can the client know which port it has to connect to when run?

[zaksnet]

Why the delay in the while true?

The delay was added because without it the connection was failing if it was too fast after the first (There is no delay if the first try works so i figured it doesn't matter). If you test the PR without the delay, re-connection for the first couple ports will fail.

How can the client know which port it has to connect to when run?

The game knows the debugging port because the code is changing the settings of the editor:
EditorSettings::get_singleton()->set("network/debug/remote_port", remote_port);
And also the play_pressed signal is emitted before the game has started so that the client can get the new settings.

EDIT: @Faless One thing to notice is that if the user opens the Editor settings after the port has changed, the settings will be saved and the port will remain to the changed value. I couldn't figure out a better way so far to do this without having to change 10 files and this seemed to work out of the box without too much hassle. Any suggestions are welcomed!

EDIT: @Faless Any ideas why server->listen(remote_port) is returning OK even if the port already in use in windows?

[zaksnet]

@Faless @akien-mga This could also be solved by making the Remote debugger port a Per project setting instead of an Editor setting. Unfortunately i don't have time to do this myself ATM but if none takes the time to do it, i will look into it.

[Faless]

I'm not sure how putting it in the project settings would help, the problem is that the debugger server doesn't know anything about what (if anything) is being launched and has no way to communicate back to the editor. I'm planning to work on this, there's also work in #33925 to expose the preset currently being launched to the debugger (since we need the export platform) so that object might be used to pass the information between the two. Additionally, after #36244 I would like to have a generic way to start the debugger (without the need to start the game at all), so we would still need a generic editor debugger port.

…

On Mon, Feb 17, 2020, 15:30 Zak Stam ***@***.***> wrote: @Faless <https://github.com/Faless> @akien-mga <https://github.com/akien-mga> This could also be solved by making the Remote debugger port a *Per project* setting instead of an *Editor* setting. Unfortunately i don't have time to do this myself ATM but if none takes the time to do it, i will look into it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34651?email_source=notifications&email_token=AAM4C3UQ2U7P2BBT3DF3OVTRDKNQZA5CNFSM4KANAUKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL6TSYI#issuecomment-587020641>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM4C3UQT5V4BQNJVL52A7TRDKNQZANCNFSM4KANAUKA> .

[zaksnet]

I'm not sure how putting it in the project settings would help, the problem is that the debugger server doesn't know anything about what (if anything) is being launched and has no way to communicate back to the editor. I'm planning to work on this, there's also work in #33925 to expose the preset currently being launched to the debugger (since we need the export platform) so that object might be used to pass the information between the two. Additionally, after #36244 I would like to have a generic way to start the debugger (without the need to start the game at all), so we would still need a generic editor debugger port.
…
On Mon, Feb 17, 2020, 15:30 Zak Stam @.***> wrote: @Faless https://github.com/Faless @akien-mga https://github.com/akien-mga This could also be solved by making the Remote debugger port a Per project setting instead of an Editor setting. Unfortunately i don't have time to do this myself ATM but if none takes the time to do it, i will look into it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34651?email_source=notifications&email_token=AAM4C3UQ2U7P2BBT3DF3OVTRDKNQZA5CNFSM4KANAUKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL6TSYI#issuecomment-587020641>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4C3UQT5V4BQNJVL52A7TRDKNQZANCNFSM4KANAUKA .

Here is my situation:
I have two different projects. One is the client and one is the server. Most of the times i have to debug these projects at the same machine, which means that i have to run two different instances of Godot Editor.
In this situation both editor debuggers will try to listen on the same port and also the game that will be launched from those debugger will try to connect at the same port on both editors. To solve this, i have to change the debugger port for one of the projects each time i need to debug them.

Unless i am misunderstanding, #36244 is solving the need of running the same game inside the same editor multiple times. I don't see how this would solve my situation (unless you are planing on making the debugger as a standalone? Or somehow use the debugger of one editor for both projects?).

Puting the debugger port in the project settings instead will allow me to assign different port for each project and wont have to go through changing it every time (of course the "Automatic" part is gone in this case but atleast this can solve the problem with an one time setting).

[Calinou]

Would it work if the editor debugging port was set to a random port in a predefined range on every startup? Each instance would automatically use a different port this way. The port would still be the same across game restarts, as long as you don't close the editor.

[zaksnet]

Would it work if the editor debugging port was set to a random port on every startup? Each instance would automatically use a different port this way. The port would still be the same across game restarts, as long as you don't close the editor.

Yeah that would work i guess. But how can you make sure you are not using a port that is already in use (by another program i mean)?

EDIT: Maybe a port range in settings instead of a single port? And then each editor can check inside godot user settings folder to see what port is already assigned? But that sounds like more work than any of the other cases.

[Faless]

As I mentioned, the main problem right now is that the debugger server doesn't know anything about what (if anything) is being launched and has no way to communicate back to the editor.
Additionally, the editor node do not start the debugger directly, but the debugger starts via signal, so there is no way right now it can communicate back the port to use to the game that will run.
Knowing if another program uses a tcp port is as easy as trying to listen to it, we can have a simple:

int port = MIN_PORT;
while server->listen(port) != OK && port < MAX_PORT:
    port += 1;

The problem, again, is communicating that properly to the game being lunched.

[zaksnet]

As I mentioned, the main problem right now is that the debugger server doesn't know anything about what (if anything) is being launched and has no way to communicate back to the editor.

I'm confused by that. Is the game that is being run using a debugger server as you say? By looking at editor/script_editor_debugger.cpp the debugger server is starting through the Editor not through the game. If i understand correctly, what you mean is that the debugger can not tell the game which port to use in order to communicate with each other. The game is loading the port from the editors settings file. That is what this PR is trying to solve by:

1. Making sure the editor/script_editor_debugger.cpp is starting on a free port by doing something similar to the code you provided.
2. Starting the debugger BEFORE(Important!) the game and saving the new port just so that when the game starts it picks up the new port.

A few points to make things clearer (or help you correct me if i'm wrong):

• The Remote Debugger Server is starting through the editor and not the game as seen in editor/script_editor_debugger.cpp and picks the port to use by getting the editor setting remote_port ((int)EditorSettings::get_singleton()->get("network/debug/remote_port");) from %appData%/Godot/editor_settings-3.tres (windows).
• The game is loading the port from the same file at %appData%/Godot/editor_settings-3.tres
• So all this PR is doing, is change the remote_port value before the game starts so that it can pick up the new port once it starts.

The only problem i had was that the code you provided is not detecting that the port is in use in windows. Here is the relevant issue thread #35350 with a test project attached. Fixed by #36321

[Faless]

To be honest, I don't like the idea of changing an editor setting just because we can't listen to a port.
It will also keep increasing every time if fails to listen.
This seems to be more a hack, I think we need a proper fix.

[zaksnet]

To be honest, I don't like the idea of changing an editor setting just because we can't listen to a port.
It will also keep increasing every time if fails to listen.
This seems to be more a hack, I think we need a proper fix.

Indeed this is more of a hack than a proper fix but i didn't see any other easy way to overcome the issue. But as far as 3.2 Stable branch is concerned here are my points:

• This seems to be working/fixing the case i described without breaking anything in any other cases.

• It will also keep increasing every time if fails to listen.

As of this, if you check the code in the PR it only retries for 6 times and then it gives up with an error:

	int remote_port = (int)EditorSettings::get_singleton()->get("network/debug/remote_port");
	int max_tries = 6; // Maybe expose this so that users can increase it?
	int current_try = 0;
	int delay = 0;

	while (true) {
		if (delay >= 1000) {
			if (server->listen(remote_port) != OK) {
				EditorNode::get_log()->add_message(String("Remote debugger failed listening on port: ") + itos(remote_port) + String(" Retrying on new port: " + itos(remote_port + 1)), EditorLog::MSG_TYPE_WARNING);
				remote_port++;
				current_try++;
			} else {
				EditorSettings::get_singleton()->set("network/debug/remote_port", remote_port);
				break;
			}

			if (current_try >= max_tries) {
				EditorNode::get_log()->add_message(String("Remote debugger error listening for connections. No free port"), EditorLog::MSG_TYPE_ERROR);
				break;
			}
			delay = 0;
		}

		delay++;
	}

• I don't see any downsides of merging this in 3.2 Stable and then you can create a proper fix for 4.0.

If you still think this is not worth it then let me so i can close the PR.

[Faless]

I was thinking something more like ( 1b06c6f ) i.e. that EditorRun asks the debugger (ScriptEditor::get_singleton()->get_debugger() in 3.2) for the connection string.
Still a bit hacky, and the debugging editor options need to be reviewed in general, but at least it doesn't mess with the editor settings.
Something like this, targeting 3.2 could probably be accepted.

[zaksnet]

I was thinking something more like ( 1b06c6f ) i.e. that EditorRun asks the debugger (ScriptEditor::get_singleton()->get_debugger() in 3.2) for the connection string.
Still a bit hacky, and the debugging editor options need to be reviewed in general, but at least it doesn't mess with the editor settings.
Something like this, targeting 3.2 could probably be accepted.

Cool, i'l make the changes.

Still a bit hacky, and the debugging editor options need to be reviewed in general, but at least it doesn't mess with the editor settings.

Can you please elaborate on this one. Is it relevant to this PR (Should i check something specific in this PR?) or is it a general observation?

[Faless]

Can you please elaborate on this one. Is it relevant to this PR (Should i check something specific in this PR?) or is it a general observation?

More a general observation, we need to double check that emitting play_pressed earlier does not cause regressions, and as mentioned we should probably review the editor settings (but that should be material for 4.0, so don't worry about that).

[aaronfranke]

To rebase, follow the PR workflow article, or if you prefer a graphical client, check out my Git workflow for Godot video. If you aren't sure how to resolve a particular conflict, resolve in favor of master, then re-add your changes manually.

EDIT: Is it ok if i close this and reopen it?

I recommend you don't do this, a force push while the PR is closed will break the PR (GitHub bug).

[zaksnet]

To rebase, follow the PR workflow article, or if you prefer a graphical client, check out my Git workflow for Godot video. If you aren't sure how to resolve a particular conflict, resolve in favor of master, then re-add your changes manually.

EDIT: Is it ok if i close this and reopen it?

I recommend you don't do this, a force push while the PR is closed will break the PR (GitHub bug).

After doing git pull upstream master and fixing the conflicts it is asking to merge. Should i go merge? NVM that. I was trying to merge with master while i have to merge with 3.2 .
So i did git pull --rebase upstream 3.2 but when i push it still adds all the other commits to this PR 😞

[zaksnet]

I'm not even sure how this was closed.. Anyway, sorry for the mess. I will create a new PR with the changes.
EDIT: I think the mess was because i was trying to rebase on 3.2 while the master is 4.0? Anyway, master has some of the files deleted so i could not resolve the conflicts and i did not know how to change the branch to 3.2.

@aaronfranke Thanks for the help!