-
-
Notifications
You must be signed in to change notification settings - Fork 22.1k
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
Fix VS project creation #66718
Fix VS project creation #66718
Conversation
Our python formatter ( I'd suggest moving the length calculation to a local variable to keep it to 2 lines instead of what the formatter wants to do here. Please edit the commit with |
Done. Glad I'm not the only one that prefers to avoid line breaks (even more so in Python). |
Maybe |
Changed the logic after running into a problem with binary output names. Names were created for dev_build=yes and =no, but since variant names were identical and collapsed to only 3, the binary would always be named .dev, even for non-dev builds. The new version only creates the names needed depending on whether dev_build is set or not, rather than creating duplicate variants that get filtered out by VS. It seems like the NMake command line is also only based on the env variable. A cleaner fix could involve: -make "dev" part of the variant names, increasing the number from 3 to 6 configurations (if it doesn't create any issues in other parts). |
That seems fine if that's the expected workflow for VS users. I never use it so I don't know which is preferred. My intention was that users would be able to select whether they want to build and run a dev build from a drop-down list, like they can choose if they want to build the editor, release template or debug template, i.e.:
And thus, like But I'm also fine with the current solution for the time being, we can reassess if users do want something like what I list above. |
Thanks! And congrats for your first merged Godot contribution 🎉 |
Those six options were my first attempt, but as Eric pointed out, that is causing issues. There seem to be some unexpected dependencies on the names. I also have to correct myself on duplicating the command line after trying to figure out how to create different ones. I only see the one and cmdargs seems to be the way to pass different additional options to scons. So I agree that this is more of a hotfix. If handling all 6 configs can be done with relatively small changes, I might open another PR, but I'm hesitant to mess around too much without a deeper understanding how the whole build system works together (especially across platforms I don't have and can't test). |
Fixes #66664.
The introduction of a dev option created a mismatch between number of build targets and variants, causing the VS project creation to fail.
PR creates the missing variants for dev/non-dev.