Visual Studio vsproj generation broken after #66242

[akien-mga]

Godot version

4.0.beta (

System information

Windows 10 & 11, Visual Studio

Issue description

As expected in #66242 since I developed it on Linux and didn't get to test on Windows/MSVC, the vsproj=yes option broke.

Here's the error that @EricEzaM reported:

[Initial build] scons: *** [godot.vcxproj] InternalError : Sizes of 'buildtarget' and 'variant' lists must be the same.
Traceback (most recent call last):
  File "C:\Users\Eric\AppData\Local\Programs\Python\Python38\lib\site-packages\SCons\Action.py", line 1318, in execute
    result = self.execfunction(target=target, source=rsources, env=env)
  File "C:\Users\Eric\AppData\Local\Programs\Python\Python38\lib\site-packages\SCons\Tool\msvs.py", line 1845, in GenerateProject
    GenerateDSP(dspfile, source, env)
  File "C:\Users\Eric\AppData\Local\Programs\Python\Python38\lib\site-packages\SCons\Tool\msvs.py", line 1795, in GenerateDSP
    g = _GenerateV10DSP(dspfile, source, env)
  File "C:\Users\Eric\AppData\Local\Programs\Python\Python38\lib\site-packages\SCons\Tool\msvs.py", line 1196, in __init__
    _DSPGenerator.__init__(self, dspfile, source, env)
  File "C:\Users\Eric\AppData\Local\Programs\Python\Python38\lib\site-packages\SCons\Tool\msvs.py", line 448, in __init__
    raise SCons.Errors.InternalError("Sizes of 'buildtarget' and 'variant' lists must be the same.")
SCons.Errors.InternalError: Sizes of 'buildtarget' and 'variant' lists must be the same.
scons: building terminated because of errors.

I'm not familiar with the way this feature implemented nor with how it's used as I almost never use Visual Studio, so it would be great if a VS user could have a look at fixing this, and making sure that the set of build configs defined in #66242 makes sense for contributors and users making custom builds.

Steps to reproduce

• Install Windows 10 or 11 with VS 2019 or 2022
• scons vsproj=yes

Minimal reproduction project

No response

[Shnazzy]

Seems it has to do with there being .dev equivalents of all of the build targets:

I put a breakpoint at the line in Scons where the first error message occurs (the one about Sizes of 'buildtarget' and 'variant' lists must be the same.), and checked what it had a problem with. Seems that the env['buildtarget'] list and the variants list aren't the same size and it has a problem with that.

pprint(env['buildtarget'])
['bin\\godot.windows.editor.x86_32.exe',
 'bin\\godot.windows.editor.dev.x86_32.exe',
 'bin\\godot.windows.editor.x86_64.exe',
 'bin\\godot.windows.editor.dev.x86_64.exe',
 'bin\\godot.windows.template_release.x86_32.exe',
 'bin\\godot.windows.template_release.dev.x86_32.exe',
 'bin\\godot.windows.template_release.x86_64.exe',
 'bin\\godot.windows.template_release.dev.x86_64.exe',
 'bin\\godot.windows.template_debug.x86_32.exe',
 'bin\\godot.windows.template_debug.dev.x86_32.exe',
 'bin\\godot.windows.template_debug.x86_64.exe',
 'bin\\godot.windows.template_debug.dev.x86_64.exe']

(Pdb) pprint(variants)
['editor|Win32',
 'editor|x64',
 'template_release|Win32',
 'template_release|x64',
 'template_debug|Win32',
 'template_debug|x64']

[afestini]

I locally added the .dev versions to the variants in methods.py

             @staticmethod
             def for_every_variant(value):
-                return [value for _ in range(len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS))]
+                return [value for _ in range(len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS) * len(ModuleConfigs.DEV_SUFFIX))]


                 self.arg_dict["variant"] += [
-                    f'{config}{f"_[{name}]" if name else ""}|{platform}'
+                    f'{config}{dev}{f"_[{name}]" if name else ""}|{platform}'
                     for config in ModuleConfigs.CONFIGURATIONS
                     for platform in ModuleConfigs.PLATFORMS
+                    for dev in ModuleConfigs.DEV_SUFFIX

It's building again, but being completely unfamiliar with the build system, that might be the completely wrong way to fix it. I also still have to double check that it's actually building the right thing.

Edit: judging from the additional build options in VS, the part of actually setting the optimization level is not working with just the above changes.

[EricEzaM]

That change also worked for me except one part - don't include {dev} in the 'variant' name, otherwise scons will not work as e.g. editor.dev is not a valid target (while editor is). Here is my diff

@@ -745,7 +745,7 @@ def generate_vs_project(env, num_jobs):

             @staticmethod
             def for_every_variant(value):
-                return [value for _ in range(len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS))]
+                return [value for _ in range(len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS) * len(ModuleConfigs.DEV_SUFFIX))]

             def __init__(self):
                 shared_targets_array = []
@@ -774,6 +774,7 @@ def generate_vs_project(env, num_jobs):
                     f'{config}{f"_[{name}]" if name else ""}|{platform}'
                     for config in ModuleConfigs.CONFIGURATIONS
                     for platform in ModuleConfigs.PLATFORMS
+                    for dev in ModuleConfigs.DEV_SUFFIX
                 ]

[afestini]

Seems I was overly worried about the resulting VS configurations. I thought using the same name would result in a bunch of duplicate configurations in VS, but while distinct names do produce 6 configs, your version only creates the 3 that actually matter.

[akien-mga]

Looks good to me, feel free to PR that change.

[afestini]

Not sure if you referred to EricEzaM or me, but just in case I created a PR.

[Geekotron]

The PR may still need a slight tweak. When I used it , the vcxproj still has .dev in the exe names:

<NMakeOutput Condition="'$(Configuration)|$(Platform)'=='editor|Win32'">bin\godot.windows.editor.dev.x86_32.exe</NMakeOutput>

... which doesn't match the exe that gets built by scons, so VS fails to find/launch the newly built exe.

Not related to the changes in this PR I think but additionally - even after I removed .dev from the setting above so that VS could launch the exe, I couldn't debug due to "module was built without debug symbols". I had to also add "debug_symbols=1" into the NMakeBuildCommandLine so that scons would create a pdb. After that I'm able to build & debug 4.0 beta 2 in VS 2022.

[EricEzaM]

Running scons from a cmd window generates the same file for me - no discrepancies between vsproj and scons via cmd. Make sure you have dev_build=yes when running from scons, as the vsproj has that. dev_build automatically includes debug_symbols.

scons p=windows dev_build=yes -j15 -> bin\godot.windows.editor.dev.x86_64.exe

<NMakeOutput Condition="'$(Configuration)|$(Platform)'=='editor|x64'">bin\godot.windows.editor.dev.x86_64.exe</NMakeOutput>

[afestini]

This might turn into a conceptual question. For ease of use I'd prefer to create all 6 configurations, but if I understand the code correctly, all configs use the same command line options and whether to use dev_build depends on just the environment variable.

Somebody with a better overview of how things tie together correct me here, but I see two options:

-Instead of adding duplicate variants to match the build target count, I should have reduced the build targets (make the names depend on the env variable in the same way as the NMake command line, rather than creating all 6 options). Downside is that changing between dev/non-dev requires recreating the project file.

-The user friendly version: create all combinations as VS configurations. That might require some bigger changes to how the command line and config names are constructed.

The patch for option 1 would be something like:

@@ -741,7 +741,7 @@ def generate_vs_project(env, num_jobs):
             PLATFORMS = ["Win32", "x64"]
             PLATFORM_IDS = ["x86_32", "x86_64"]
             CONFIGURATIONS = ["editor", "template_release", "template_debug"]
-            DEV_SUFFIX = ["", ".dev"]
+            DEV_SUFFIX = ".dev" if env["dev_build"] else ""
 
             @staticmethod
             def for_every_variant(value):
@@ -776,10 +776,9 @@ def generate_vs_project(env, num_jobs):
                     for platform in ModuleConfigs.PLATFORMS
                 ]
                 self.arg_dict["runfile"] += [
-                    f'bin\\godot.windows.{config}{dev}.{plat_id}{f".{name}" if name else ""}.exe'
+                    f'bin\\godot.windows.{config}{ModuleConfigs.DEV_SUFFIX}.{plat_id}{f".{name}" if name else ""}.exe'
                     for config in ModuleConfigs.CONFIGURATIONS
                     for plat_id in ModuleConfigs.PLATFORM_IDS
-                    for dev in ModuleConfigs.DEV_SUFFIX
                 ]

[Geekotron]

Thanks @EricEzaM , I was indeed missing dev_build in my env. I guess I missed an instruction somewhere in the scons/visual studio setup guide. After adding that, the vcxproj resulting from this PR change worked fine for debugging.

[EricEzaM]

@afestini that code diff does appear to make more sense, if the end result is the same.

I don't think all combinations are needed in the vsproj. The assumption I think is that if you are using vsproj, you are in development and thus want the dev build. If you want a nondev build it is easy enough do do that from the terminal.

[afestini]

@EricEzaM that's a good point I lost sight off. Since the project still goes through Scons, there is not much point in creating one if you're not going to debug. I usually build projects through VS so I can easily check all the options and settings (and potentially tweak them). That doesn't really apply here.

Looking at the docs, the description for VS is in fact focusing on debugging and the current approach would change the command line to achieve that (meaning, require updating the documentation). Would we create any inconsistencies by making dev_build the default and require an explicit dev_build=no otherwise?

[EricEzaM]

I am pretty sure that previously the vsproj build command included the commands to essentially do dev_build=yes

[afestini]

Going through the original change and the description, the second version would be in line with how I understand it, specifically:

Note: Unlike previously, dev_build defaults to off so that users who
compile Godot from source get an optimized and small build by default.
Engine contributors should now set dev_build=yes in their build scripts or
IDE configuration manually.

While it's arguably a less useful default for the VS project creation, it might still be the least surprising and consistent with the original PR.

I played through the scenarios of both versions and the first (current PR) always places .dev in the binary name, while the actual build option is only set when using dev_build=yes during project creation. The second one seems to behave the same, except with the correct binary name. So I will update my PR with the second version.

[afestini]

In case anyone actively using VS is still interested, I have one more and (maybe) final version that creates all six configurations ("editor", "editor (dev build)", etc.). Instead of making VS mess with the config name to extract the target, it uses the targets directly to create different cmdargs for each variant. It somewhat triggers my OCD by moving target= and dev_build= to the end of the command line, but otherwise seems to work. Side effect: the values of "variant" can be modified based on what you want to show in VS without worries about breaking the scons target names.

@@ -742,11 +742,12 @@ def generate_vs_project(env, num_jobs):
             PLATFORMS = ["Win32", "x64"]
             PLATFORM_IDS = ["x86_32", "x86_64"]
             CONFIGURATIONS = ["editor", "template_release", "template_debug"]
-            DEV_SUFFIX = ".dev" if env["dev_build"] else ""
+            DEV_SUFFIX = ["", ".dev"]
 
             @staticmethod
             def for_every_variant(value):
-                return [value for _ in range(len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS))]
+                count = len(ModuleConfigs.CONFIGURATIONS) * len(ModuleConfigs.PLATFORMS) * len(ModuleConfigs.DEV_SUFFIX)
+                return [value for _ in range(count)]
 
             def __init__(self):
                 shared_targets_array = []
@@ -772,27 +773,26 @@ def generate_vs_project(env, num_jobs):
                     defines = []
                 self.names.append(name)
                 self.arg_dict["variant"] += [
-                    f'{config}{f"_[{name}]" if name else ""}|{platform}'
+                    f'{config}{f" (dev build)" if dev else ""}{f"_[{name}]" if name else ""}|{platform}'
                     for config in ModuleConfigs.CONFIGURATIONS
                     for platform in ModuleConfigs.PLATFORMS
+                    for dev in ModuleConfigs.DEV_SUFFIX
                 ]
                 self.arg_dict["runfile"] += [
-                    f'bin\\godot.windows.{config}{ModuleConfigs.DEV_SUFFIX}.{plat_id}{f".{name}" if name else ""}.exe'
+                    f'bin\\godot.windows.{config}{dev}.{plat_id}{f".{name}" if name else ""}.exe'
                     for config in ModuleConfigs.CONFIGURATIONS
                     for plat_id in ModuleConfigs.PLATFORM_IDS
+                    for dev in ModuleConfigs.DEV_SUFFIX
                 ]
                 self.arg_dict["cpppaths"] += ModuleConfigs.for_every_variant(env["CPPPATH"] + [includes])
                 self.arg_dict["cppdefines"] += ModuleConfigs.for_every_variant(env["CPPDEFINES"] + defines)
-                self.arg_dict["cmdargs"] += ModuleConfigs.for_every_variant(cli_args)
+                self.arg_dict["cmdargs"] += [
+                    f'target={config}{" dev_build=yes" if dev else ""}{" {cli_args}" if cli_args else ""}'
+                    for config in ModuleConfigs.CONFIGURATIONS
+                    for _ in ModuleConfigs.PLATFORMS
+                    for dev in ModuleConfigs.DEV_SUFFIX]
 
             def build_commandline(self, commands):
-                configuration_getter = (
-                    "$(Configuration"
-                    + "".join([f'.Replace("{name}", "")' for name in self.names[1:]])
-                    + '.Replace("_[]", "")'
-                    + ")"
-                )
-
                 common_build_prefix = [
                     'cmd /V /C set "plat=$(PlatformTarget)"',
                     '(if "$(PlatformTarget)"=="x64" (set "plat=x86_amd64"))',
@@ -806,7 +806,6 @@ def generate_vs_project(env, num_jobs):
                 common_build_postfix = [
                     "--directory=\"$(ProjectDir.TrimEnd('\\'))\"",
                     "platform=windows",
-                    f"target={configuration_getter}",
                     "progress=no",
                     "-j%s" % num_jobs,
                 ]

[RicardRC]

In case anyone actively using VS is still interested, I have one more and (maybe) final version that creates all six configurations ("editor", "editor (dev build)", etc.). Instead of making VS mess with the config name to extract the target, it uses the targets directly to create different cmdargs for each variant. It somewhat triggers my OCD by moving target= and dev_build= to the end of the command line, but otherwise seems to work. Side effect: the values of "variant" can be modified based on what you want to show in VS without worries about breaking the scons target names.

I am interested in this, having to regenerate visualstudio solution when debugging custom modules (the core of my game, actually...) is a pain, and running my game in zero-optimization that dev build enables is also a pain.