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

Detect and update .csproj when C# files are added, moved or removed. #20287

Closed
wants to merge 1 commit into from

Conversation

Nufflee
Copy link
Contributor

@Nufflee Nufflee commented Jul 19, 2018

The .csproj file is updated when the C# solution is built to also account for changes outside the Godot editor. I also updated the .gitignore file to ignore .vscode directories in any directory, not just the root.

This closes #12917 and closes #12415.

@neikeq
Copy link
Contributor

neikeq commented Jul 19, 2018

The problem with this is it will add every .cs file inside the project directory to the main C# project, even if the user removed it explicitly, or if it's part of another C# project.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@neikeq Yeah, that's true. But is it such a big problem? How would you suggest fixing it?

@Zylann
Copy link
Contributor

Zylann commented Jul 20, 2018

Same as @neikeq, solving it by regenerating the solution from scratch is a bad idea and must be handled with care, as I explained here #12415 (comment). To preserve the state of the solution, Godot should just do what it needs to do (aka add or remove files), not recreate it all.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@Zylann But this is not overriding the solution really. It only removes the <Compile> tag from the .csrpoj and leaves all other settings untouched. It also only does that on build. Most IDE's do it this way.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

I could be missing something though.

@Zylann
Copy link
Contributor

Zylann commented Jul 20, 2018

@Nufflee if it only adds or remove files that's fine, but as @neikeq said it could conflict with other C# projects within the Godot project.
For excluded files, I can't think of a solution that doesn't involve knowing which files were moved, it's kind of an annoying case since such C# files would be garbage to be cleaned anyways.

On a side note I wonder if editor-only assemblies were considered yet, but those would typically be in different projects as well (unless Godot does something special).

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@Zylann @neikeq What Unity does to solve this issue is that they only allow 3 (or 4 but doesn't really matter) assemblies per project with each one of them being in a special directory.
Editor (only) assembly is in Editor directory, Plugins assembly in Plugins and the main (game) assembly is everything else.

We should probably do this though it limits the number of game assemblies. We can somehow map assemblies to directories to know which .cs file belongs to which assembly and not include an option for excluded files. This is the best solution I can think of.

@Zylann
Copy link
Contributor

Zylann commented Jul 20, 2018

@Nufflee Marking folders for custom projects can also be a solution indeed.
Out of curiosity, research led me here: http://devleader.ca/2015/02/08/multiple-c-projects-unity-3d-solution/

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@Zylann From what I can see, the person in the article just let Visual Studio manage .csproj files instead of Unity as it does that better.

So the solution would be to, when a .cs file is found, we go up the directory tree until we find a .csproj file. When we find it we just add the .cs file to it.
When checking for removed files, we can just find all ,csproj files and check all .cs files down the directory tree and remove them if needed.
It's extremely rare to have project files outside of the directory containing the .csproj file if that's even possible.

I hope this makes sense as it almost 4 AM.

@Zylann
Copy link
Contributor

Zylann commented Jul 20, 2018

Last concern I just thought: walking the whole project can take a lot of time as it grows, so this should happen only if .cs files were added/removed, not at every build (incremental build could end up being faster than the scan). But I wonder if there is a way for the build system to know that.

@NathanWarden
Copy link
Contributor

Unity created a new way of defining solutions around last November called assembly definition files. This allows you to place a definition asset within a directory and it will generate a solution for that directory. If there's another assembly def under that directory it will use that instead. Then, as options on the assembly defs you can define which platforms that solution should compile for such as: Editor Only, Windows, Mac, Linux, iOS, Android, etc. You can reference other assembly definitions on another assembly def in order to define dependencies. It's very straight forward and quite flexible.

I wonder if there's some sort of hybrid approach, because I can see the need for wanting to have very meticulous access the the csproj and sln files, but I can also see this becoming a debugging nightmare due to using slightly incorrect settings. For instance, last I checked (it was a few months ago) we have to change 4 different things in order to simply change the name of a Godot game project in order for it to compile again. An auto-generated solution wouldn't have this problem at all.

But, I suppose this is a conversation for another thread at another time, but I think it would be worth exploring down the road. IE. maybe having the sln/csproj files auto-generated by default, but we could override it by pointing it to a specific sln file in the project settings.

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

@Zylann I'm not too sure how slow would it be but I made it walk to whole solution to also account for file system changes (basically those made from a code editor like VS Code even though this could also be quite easily handled on VS Code level but it's not the only code edtor people use so it wouldn't be a very universal solution).
We could use a file watcher for this though? I don't know what @neikeq thinks about this. Is using a file watcher over-engineering?

@Nufflee
Copy link
Contributor Author

Nufflee commented Jul 20, 2018

And yeah, @NathanWarden, you should probably open an issue for that.

@redxdev
Copy link

redxdev commented Aug 16, 2018

I'm also of the opinion that godot shouldn't be automatically screwing with the project files - leave that to IDEs which specialize in it. I think that updating the project file only when adding/removing/moving scripts via the godot editor itself makes more sense than doing a filesystem scan on build. The benefit is that when a user adds a script via the editor, it's fairly obvious that the intent is to add it to the godot project. When a file is added outside of the godot editor, it might be for something else entirely.

@akien-mga
Copy link
Member

As per the above discussion, it seems that this wouldn't be the proper solution to the original problem, so closing. Thanks nevertheless for the proposal and for the discussion it started!

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