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

Automatic remote debugger port assignment. #37067

Merged
merged 1 commit into from
May 1, 2021

Conversation

zaksnet
Copy link
Contributor

@zaksnet zaksnet commented Mar 15, 2020

This is the continuation of #34651. This PR is only relevant to 3.2 as 4.0 has undergone many changes and it would require a different approach.

I'v made the changes requested by @Faless in #34651 and the game is now requesting the port from the debugger instead of saving/loading it.

image

@wojtossfm
Copy link

While waiting for this to hit 3.2 anyone who has a similar issue can work around this as far as I can tell by adding a tailored plugin to each of their projects e.g. (with an altered range)

tool
extends EditorPlugin


const RANGE_MIN = 10100
const RANGE_MAX = 10200

func build():
	var interface = self.get_editor_interface()
	var settings = interface.get_editor_settings()
	var port = randi() % RANGE_MAX + RANGE_MIN
	settings.set('network/debug/remote_port', port)
	prints("Set port to in settings to", port)
	return true

Managing to consistently get my client/server to connect to their respective debuggers when launching from the editor. It might be overly complex but it works for me so figured I would share in case someone is running into a similar issue.

@akien-mga
Copy link
Member

CC @Faless

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Some necromancy here, should have reviewed this time ago.
I still fear this feature will be confusing, but it's good to go after removing the extra message (see above).
👍

editor/script_editor_debugger.cpp Outdated Show resolved Hide resolved
@zaksnet zaksnet requested a review from a team as a code owner February 27, 2021 16:46
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@akien-mga
Copy link
Member

The commit message should be amended, it doesn't say at all what the commit is about.

@akien-mga
Copy link
Member

And this targets 3.2, what about the master branch?

@zaksnet
Copy link
Contributor Author

zaksnet commented Mar 2, 2021

And this targets 3.2, what about the master branch?

I'm afraid that this can not be applied to 4.0 since there has been many structural changes there.

@zaksnet
Copy link
Contributor Author

zaksnet commented Mar 4, 2021

The commit message should be amended, it doesn't say at all what the commit is about.

Added more info about the commit in the description 👍

Base automatically changed from 3.2 to 3.x March 16, 2021 11:11
@aaronfranke aaronfranke modified the milestones: 3.2, 3.3 Mar 16, 2021
@akien-mga akien-mga removed this from the 3.3 milestone Mar 26, 2021
@akien-mga akien-mga added this to the 3.4 milestone Mar 26, 2021
@akien-mga
Copy link
Member

And this targets 3.2, what about the master branch?

I'm afraid that this can not be applied to 4.0 since there has been many structural changes there.

@Faless Would you be able to do the forward-port to 4.0?

Previously if more than one Godot editor was running then the debugger of one editor would not work because both editors were trying to connect on the same port.
This commit attempts to fix this by allowing the debugger to change the port at runtime in such cases.
@zaksnet zaksnet force-pushed the multiple-editor-instances branch from 45ddc8d to c3cfb87 Compare May 1, 2021 09:32
@akien-mga akien-mga merged commit 48cc756 into godotengine:3.x May 1, 2021
@akien-mga
Copy link
Member

Thanks!

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.

6 participants