Skip to content

Fix launch issue when GodotProjectDir not same as SolutionDir #25

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paiden
Copy link

@paiden paiden commented Feb 5, 2022

If the Solution for a Godot project is not the same as the project directory the extension will
fail to launch the debug target with a MessageBox showing 'No Godot editor instance connected'.

The reason is that the extension takes SolutionDir as the Godot project dir. While this hold true for
simple default Godot projects this must not be the case. In complex projects developers may
restructure their projects into a alternative structure, where the solution may be at another location.

The *.csproj file has the MSBuild property 'GodotProjectDir'. Now the value of this property is fetched
and will be used as the project dir.

Note: Godot itself has some issues with projects varying to the default project. To fix this godot issues,
modifications in the GodotSDK MSBuild props file have to be done.

If the Solution for a Godot project is not the same as the project directory the extension will
fail to launch the debug target with a MessageBox showing 'No Godot editor instance connected'.

The reason is that the extension takes SolutionDir as the Godot project dir. While this hold true for
simple default Godot projects this must not be the case. In complex projects developers may
restructure their projects into a alternative structure, where the solution may be at another location.

The *.csproj file has the MSBuild property 'GodotProjectDir'. Now the value of this property is fetched
and will be used as the project dir.

Note: Godot itself has some issues with projects varying to the default project. To fix this godot issues,
modifications in the GodotSDK MSBuild props file have to be done.
@paiden paiden force-pushed the fix/attach-when-project-in-sub-directory branch from a1f81dd to 814c65c Compare February 5, 2022 16:54
@neikeq
Copy link
Contributor

neikeq commented Feb 5, 2022

Currently, Godot doesn't support this. It expects both the solution and csproj to be at the Godot project root (next to the project.godot). I plan to change this for Godot 4.0, but it's unlikely to ever change for Godot 3.x.

Note: Godot itself has some issues with projects varying to the default project. To fix this godot issues,
modifications in the GodotSDK MSBuild props file have to be done.

Yes, the Sdk makes these assumptions because the Godot editor does.

@paiden
Copy link
Author

paiden commented Feb 6, 2022

So what does this mean for this PR? Do you consider merging it? Or not, because this fix is currently not needed?

IMO this way of resolving the GodotProjectDir is the more MSBuild like way of doing so (considering MSBuild does not know what Solutions are 😉 ). Also while not needed for Gotdot3 this fix should be fully compatible and not break anything, but should also work with Godot versions that allow a more flexible Project structure in the future (as long as the property name stays the same).

BTW: Godot3 may not officially support SlnDir!=ProjDir. The workaround to make it work was to this small change

    <GodotProjectDir Condition=" '$(SolutionDir)' != '' ">$(SolutionDir)</GodotProjectDir>
    <GodotProjectDir Condition=" '$(SolutionDir)' == '' ">$(MSBuildProjectDirectory)</GodotProjectDir>

to

    <GodotProjectDir>$(MSBuildProjectDirectory)</GodotProjectDir>

I did not test everyting, but all I need for now (some custom nodes, addins etc.) work fine as far as I can tell.

Note that the first XML fragment is a little weird under the assumption that SolutionDir == ProjectDir. In that case you can just use ProjectDir that always exists anyway. It will have the same value as SolutionDir but unlike that one, it is always defined.

Its fine if you say you will not merge this. In that case sorry for the trouble. I did not find any contribution guidelines that define what the PR process is. If so, I will make a forked version of the addin, where I will also do a second fix for #10.

@BenMcLean
Copy link

BenMcLean commented Feb 6, 2022

Godot3 may not officially support SlnDir!=ProjDir.

What I do with Godot Mono is this:

In my object tree in the Godot editor, I have one object, called Main and I attach to it one script, called Main.cs. Everything else goes in folders and is built entirely from code at runtime.

I understand that this is not the way Godot was designed to work because it's designed to be a game engine that takes care of everything for you, like Unity or Unreal and I'm forcing it to work more like a framework such as libGDX. But I didn't start form the assumption that I wanted Godot: I started from the assumption that I wanted open source, C#, VR support and spend as little time as possible on low level hardware stuff so Godot was the only option at the time. I'd probably go with StereoKit or something else if I was starting from scratch right now. (no offense to Godot)

Anyway, my point is that your project can minimally conform to the standard and still work.

@neikeq
Copy link
Contributor

neikeq commented Feb 7, 2022

I may have misunderstood this. Is it only about the solution being on a different directory and not the csproj? The Godot editor still expects a solution to be in the project root, but I suppose you have another one you use in Visual Studio. That should be possible then, so this change makes sense.
We can update the Sdk too, although you can already make this work by adding <GodotProjectDir>$(MSBuildProjectDirectory)</GodotProjectDir> in a Directory.Build.props.

@paiden
Copy link
Author

paiden commented Feb 7, 2022

Currently the extension does this:

// Pseudocode
_godotProjectDir = ActiveSolution.Directory

A project for that IsGodotProject() is true, has a MSBuild Prop $(GodotProjectDir) that
already holds a value that represents this directory.

So in my opinion doing this:

// Pseudocode
_godotProjectDir = godotProject.$(GodotProjectDir)

is the correct/idiomatic approach to determine the project directory for a Godot
project, while the former approach works because of implementation details.

My Godot project runs completely fine in the GodotEditor. The Visual Studio
extension refuses to launch it
. The PR applies changes, so that the extension
will launch my project and keep launching all projects it could launch prior to this.
So I'd argue this is an improvement (as long as this change does not
introduce regressions).

So my impression is, this may be an up-stream worthy fix. So I opened this PR.
Now the maintainer of this project has to decide if this really
is an up-stream worthy change - weight risk of regression vs. impact of improvements ... -
and decide accordingly.

If, Godot itself supports Solutions in some other Directory, or whatever MSBuild
tricks are needed to make it work, is IMO not relevant for the PR at hand.

This is the best I can do, to articulate why I opened this PR (I'm not a native English
speaker).

@GeorgeS2019

This comment was marked as off-topic.

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

Successfully merging this pull request may close these issues.

4 participants