Refactor VS specific glob_recursive_2 function

[dustdfg]

It also:

• Deletes old unused add_to_vs_project function
• Ensures that list passed to json.dumps is properly sorted

Both these additional "bugs" were added in 7638a6c

1. add_to_vs_project function was just overlooked
2. sort_keys parameter of json.dumps doesn't have any sense when you pass there a list which doesn't have any keys... Before consistent order was provided by deterministic nature of search code and list concatenation but now it will be ensured by using sorted(). And sorted() in this PR is mandatory because when you do [*list, *set], order of elements will be random when you expand sets...

[dustdfg]

CC @shana

[shana]

This change is dropping directories without files but that have subdirectories with files in them, which means the filters file will not have complete entries and some files will be missing. Compare the output of vcxproj.filters before and after this change (sort the file in an editor for an easier comparison, and note that, for instance, the Filter entries Source Files\misc, Source Files\misc\dist and Source Files\misc\dist\ios_xcode are all missing for the path misc\dist\ios_xcode\godot_ios\dummy.cpp. These entries are required for the filtering to work, and without them, the project will have all of those files dumped into the root of the project instead of sorted into the respective folders. You should open the project after generating to see if there are any orphan files at the root of the project - there shouldn't be any loose files outside of folders.

[dustdfg]

First of all. I use Linux... Yeah, I understand what you think that I am proposing changes to vs code while not use... But this info is crucial for context of my following questions...

1. You can generate vs proj files under different platforms and they should be identical?
   1.1. If running scons vsproj="yes" under linux isn't supposed, so why does it appear possible to run it on linux!
2. (Just to be sure) Running scons vsproj="yes" must produce not only <Filter Include="Source Files\misc\dist\ios_xcode\godot_ios"> but also <Filter Include="Source Files\misc\dist\ios_xcode">?
   2.1 Are you sure that master behaves differently? Because when I run scons vsproj="yes" (obviously under linux) I see inside godot.vcxproj.filters only Source Files\misc/dist/ios_xcode/godot_ios it doesn't produce Source Files\misc
3. It should be Source Files\misc\dist\ios_xcode not Source Files\misc/dist/ios_xcode (different slahes)? In my case (underlinux) it produces mix of slashed and the correct one is the hardcoded slash...

[dustdfg]

Do you agree about add_to_vs_project function deletion and replacing sort_keys with sorted() or just deleting sort_keys without replacing because sort_keys anyway doesn't have sense?

[dustdfg]

Just thought that I forgot to look at variables of add_to_vs_project and where they are used. They are used in following places:

godot/SConstruct

Lines 1057 to 1060 in e65a237

	if env["vsproj"]:
	env.vs_incs = []
	env.vs_srcs = []

godot/platform/windows/SCsub

Lines 83 to 91 in e65a237

	# Microsoft Visual Studio Project Generation
	if env["vsproj"]:
	env.vs_srcs += ["platform/windows/" + res_file]
	env.vs_srcs += ["platform/windows/godot.natvis"]
	for x in common_win:
	env.vs_srcs += ["platform/windows/" + str(x)]
	if env["windows_subsystem"] == "gui":
	for x in common_win_wrap:
	env.vs_srcs += ["platform/windows/" + str(x)]

But looks like the only real place where they were is used is a function deleted in the same commit as in the description of PR will check later and maybe will split this PR

[shana]

@dustdfg oh sorry, totally missed your question there. Yeah, I left the code in because the new implementation was optional initially, but there's no point in keeping the old code around now, deleting is great.

As for the sorting, I didn't realize it wasn't actually sorting, so yeah, using sorted is good. Sorting is important there because we're calculating an md5 hash of all the content so we can avoid touching the file if it hasn't been changed, to avoid VS reloading projects and potentially some user customizations that might be lost in that unavoidable process, so we should keep those keys sorted.

[dustdfg]

As all fruitful and non destructive changes are extracted in a separate PR, I am closing this because see that probably refactoring will use more SCons internals than necessary, can impact performance (I don't think that much but can), possibly can't be tested by me, I am a bit tired of this concrete topic and maybe will return to it later

@shana but I would appreciate if you answer other questions which will allow me to get deeper knowledge

[shana]

• You can generate vs proj files under different platforms and they should be identical?
  1.1. If running scons vsproj="yes" under linux isn't supposed, so why does it appear possible to run it on linux!

It's totally fine to generate VS project files on Linux. You probably can't use them to build on Linux if you don't have an IDE that supports the format, but that doesn't mean you can't have a CI generate the project files and send them off with the source to a Windows machine to use (as an example).

• (Just to be sure) Running scons vsproj="yes" must produce not only <Filter Include="Source Files\misc\dist\ios_xcode\godot_ios"> but also <Filter Include="Source Files\misc\dist\ios_xcode">?

Yes, the entire folder structure of a path has to be generated, with each part of the path as a separate entry building up to the full path: Source Files\misc, Source Files\misc\dist, Source Files\misc\dist\ios_xcode, Source Files\misc\dist\ios_xcode\godot_ios).
Each with its own GUID. It's super annoying, but that's how it works

2.1 Are you sure that master behaves differently? Because when I run scons vsproj="yes" (obviously under linux) I see inside godot.vcxproj.filters only Source Files\misc/dist/ios_xcode/godot_ios it doesn't produce Source Files\misc

This is what is produced when I run it on master:

<Filter Include="Source Files\misc"><UniqueIdentifier>{c7400380-89df-426e-b362-d214d6ad5112}</UniqueIdentifier></Filter>
<Filter Include="Source Files\misc\dist"><UniqueIdentifier>{80c5bd10-89d0-4097-8af1-d933e9e74551}</UniqueIdentifier></Filter>
<Filter Include="Source Files\misc\dist\ios_xcode"><UniqueIdentifier>{1c59e56f-a910-4e6c-9fe0-160fe6a709c3}</UniqueIdentifier></Filter>
<Filter Include="Source Files\misc\dist\ios_xcode\godot_ios"><UniqueIdentifier>{918484f6-e342-45b5-be54-27c0db8b6958}</UniqueIdentifier></Filter>

...

<ClCompile Include="misc\dist\ios_xcode\godot_ios\dummy.cpp"><Filter>Source Files\misc\dist\ios_xcode\godot_ios</Filter></ClCompile>

• It should be Source Files\misc\dist\ios_xcode not Source Files\misc/dist/ios_xcode (different slahes)? In my case (underlinux) it produces mix of slashed and the correct one is the hardcoded slash...

It must be backslashes \, not forward slashes. If it's producing forward slashes on unix, then that's a bug.