Document the Unification of tools/target build type configuration #6316

NeroBurner · 2022-10-15T20:32:08Z

Introduced in godotengine/godot#66242 the tools=yes/no option was removed and merged into the target preset.

Replace the tools=yes/no calls in the documentation to the new template target presets editor, template_release and template_debug.

It's my first PR here, so please tell me if something is missing or done wrong. I just was confused why the scons target=debug builds didn't work. After finding out about the change I thought I'd take this opportunity to fix the documentation for everyone and learn quite a bit in the process

development/compiling/compiling_for_android.rst

development/compiling/compiling_for_linuxbsd.rst

development/compiling/compiling_for_macos.rst

bruvzg · 2022-10-15T20:52:56Z

development/compiling/compiling_with_mono.rst


-    scons p=<platform> tools=yes module_mono_enabled=yes mono_glue=no
+    scons p=<platform> target=editor module_mono_enabled=yes mono_glue=no


mono_glue is no longer used. Build process is:

Build executable with module_mono_enabled=yes

Run --headless --generate-mono-glue modules/mono/glue

Run ./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform={PLATFORM_NAME}

does the first binary need to be a target=editor build or can it be one of the template_debug or template_release templates as well?

Do we need special steps to create target=template_release production=yes builds?

In the current documentation examples list all the commands to create all three variants. What would be the updated equivalent?

# Build temporary binary scons p=windows target=editor module_mono_enabled=yes # Generate glue sources bin\godot.windows.editor.x86_64.mono --generate-mono-glue modules/mono/glue # Build binaries ./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform={PLATFORM_NAME} # are the below steps needed with the new commands? ### Build binaries normally # Editor scons p=windows target=editor module_mono_enabled=yes # Export templates scons p=windows target=template_debug module_mono_enabled=yes scons p=windows target=template_release module_mono_enabled=yes

development/compiling/compiling_with_mono.rst

development/compiling/optimizing_for_size.rst

gamedevsam

The docs for setting up Visual Studio Code are also incorrect at this point. I was unable to hit any breakpoints until I set the "dev_build=yes" scons option in my build task.

development/compiling/compiling_for_linuxbsd.rst

NeroBurner · 2022-10-16T09:26:54Z

I've tried to incorporate all feedback. Some things are still unclear (see above). Please re-review

development/cpp/configuring_an_ide/visual_studio_code.rst

NeroBurner · 2022-11-22T20:05:40Z

rebased to solve conflicts

development/cpp/configuring_an_ide/visual_studio_code.rst

raulsntos

I looked at the instructions for compiling with "Mono" and left a few suggestions but there are many other issues unrelated to this PR with this documentation since building with the .NET module is very different from what it was in 3.x.

Mono is no longer required as it's not used anymore, we also no longer build the Mono runtime and mobile is not currently supported (likely won't be until 4.1+). Since this grows the scope of the PR I think it's fine to ignore and open a separate PR for the .NET documentation.

Keep in mind that the process will likely change again with the editor unification so it may not be worth updating the .NET documentation for now.

raulsntos · 2022-11-22T22:14:53Z

development/compiling/compiling_with_mono.rst

@@ -58,15 +58,15 @@ Generate the glue
 Glue sources are the wrapper functions that will be called by managed methods.
 These source files must be generated before building your final binaries. In
 order to generate them, first, you must build a temporary Godot binary with the


This is no longer a temporary Godot binary it's the final one, now we only need to build Godot once.

raulsntos · 2022-11-22T22:18:29Z

development/compiling/compiling_with_mono.rst

 This path must be ``modules/mono/glue`` in the Godot directory::

-    <godot_binary> --generate-mono-glue modules/mono/glue
+    <godot_binary> --headless --generate-mono-glue modules/mono/glue

 This command will tell Godot to generate the file ``modules/mono/glue/mono_glue.gen.cpp``,


We no longer generate a mono_glue.gen.cpp file.

development/compiling/compiling_with_mono.rst

raulsntos · 2022-11-22T23:51:35Z

development/compiling/compiling_with_mono.rst

-Once you have generated the Mono glue, you can build the final binary with
-``mono_glue=yes``. This is the default value for ``mono_glue``, so you can also
-omit it. To build a Mono-enabled editor::
+Once you have generated the Mono glue, you can generate the final binary with


This doesn't generate the Godot binary, it just builds the .NET assemblies into DLLs that will be loaded by the .NET module.

We may also want to add instructions about setting up a local NuGet repository like in the module's README.md.

development/compiling/compiling_with_mono.rst

raulsntos · 2022-11-22T23:56:10Z

development/compiling/compiling_with_mono.rst


    ### Build binaries normally
    # Editor
-    scons p=windows target=release_debug tools=yes module_mono_enabled=yes
+    scons p=windows target=editor module_mono_enabled=yes


This step is no longer necessary since the first scons build is already the final binary, no need to build it again. Same for other platforms.

raulsntos · 2022-11-22T23:58:32Z

development/compiling/compiling_with_mono.rst

-    bin\godot.windows.tools.64.mono --generate-mono-glue modules/mono/glue
+    bin\godot.windows.editor.x86_64.mono --generate-mono-glue modules/mono/glue
+    # Generate binaries
+    ./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform=windows


Can you run a python script like this in Windows? I think you might need to execute it with the python command and the directory separator should probably be replaced with \.

Forward slash directory separators should be converted automatically by Windows, but the leading ./ should be removed as it won't work (it can also be removed on macOS/Linux, for files that are in a subfolder).

You will need to add the python command in front of the script path though.

raulsntos · 2022-11-24T18:35:05Z

development/compiling/compiling_with_mono.rst

-
-  - Whether to include the glue source files in the build
-    and define ``MONO_GLUE_DISABLED`` as a preprocessor macro.
-
 - **mono_prefix**\ =path


mono_prefix is now an argument of the build_assemblies.py script, although it probably should be removed as we no longer use Mono and I feel like using Mono's MSBuild may not work.
mono_static and copy_mono_root have also been removed in 4.0.

Maran23 · 2022-12-01T19:30:39Z

I looked at the instructions for compiling with "Mono" and left a few suggestions but there are many other issues unrelated to this PR with this documentation since building with the .NET module is very different from what it was in 3.x.

Mono is no longer required as it's not used anymore, we also no longer build the Mono runtime and mobile is not currently supported (likely won't be until 4.1+). Since this grows the scope of the PR I think it's fine to ignore and open a separate PR for the .NET documentation.

Keep in mind that the process will likely change again with the editor unification so it may not be worth updating the .NET documentation for now.

I'm in favor of merging this PR first and doing that work in a follow-up PR.
I wasted like an hour building the Godot 4.0 project because the documentation does not reflect the changes from the godotengine/godot#66242 PR.
We should avoid that, I almost gave up because I didn't know what to do (it worked for me before obviously and I thought the whole time it's an issue with my VSCode).

development/compiling/introduction_to_the_buildsystem.rst

development/cpp/configuring_an_ide/visual_studio_code.rst

development/compiling/introduction_to_the_buildsystem.rst

development/compiling/compiling_for_macos.rst

Maran23 · 2022-12-01T19:54:29Z

I left some comments regarding spelling and typos. Otherwise it looks fine and I want to get this merged!:)
Also it is mandatory to squash the commits together as also stated here: https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#modifying-a-pull-request

Calinou · 2022-12-01T21:23:59Z

Also it is mandatory to squash the commits together as also stated here: docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#modifying-a-pull-request

We can use Squash on merge on the godot-docs repository, so it's not necessary here.

Maran23

Looks good to me.
The Mono/.NET documentation should be updated in a follow up and was partly outdated before already.

Introduced in godotengine/godot#66242 the `tools=yes/no` option was removed and merged into the `target` preset. Replace the `tools=yes/no` calls in the documentation to the new template target presets `editor`, `template_release` and `template_debug`.

Changed mentions for `vscode`, `kdevelop` and `qtcreator`

Co-authored-by: Raul Santos <[email protected]> Co-authored-by: Marius Hanl <[email protected]>

NeroBurner · 2022-12-06T20:01:00Z

Thanks for the feedback, incorporated the non-mono parts. I'm in favor of leaving out the mono-related changes in this PR as it seems those need someone more capable to incorporate those .NET documentation changes. I also have no Windows installation to check the documented commands. So please if possible merge this PR, as I don't intend to update the mono documentation because of the mentioned reasons

Maran23

LGTM

clayjohn

Mostly looks good to me. I reviewed the introduction to buildsystem page quite closely as well as the platforms I am familiar with: Android, Linuxbsd, macOS, web, and windows.

I have a left a few suggestions for improvements. I can also see that there are a few outstanding comments for building with mono. The building with mono page seems quite complex and it may be best to have one of the mono maintainers do the update rather than including it here. I want to avoid blocking this PR any longer as these pages desperately need the update.

development/compiling/introduction_to_the_buildsystem.rst

Co-authored-by: Clay John <[email protected]>

clayjohn

Everything except the mono page seem fine to me.

We discussed this a bit on the rocketchat and agreed to move forward with this PR as-is. The Mono changes are not from godotengine/godot#66242, but from all of the other changes that have happened recently so it is fine for them to be made in a separate PR from someone who is more knowledgeable about the c sharp specific changes to the buildsystem

Maran23

Thanks!

clayjohn · 2022-12-07T22:59:12Z

Thank you very much @NeroBurner for your hard work and sticking with this PR. And congratulations on your first merged PR in this repo!

Thanks to you as well @Maran23 for your multiple diligent reviews.

Anutrix · 2023-01-03T01:49:17Z

Was dev_mode intentionally left out?

bruvzg reviewed Oct 15, 2022

View reviewed changes

clayjohn added the bug label Oct 15, 2022

gamedevsam suggested changes Oct 16, 2022

View reviewed changes

development/compiling/compiling_for_linuxbsd.rst Outdated Show resolved Hide resolved

ZoopOTheGoop mentioned this pull request Nov 10, 2022

Change build flags in Godot 4 docs to current variants #6375

Closed

Maran23 reviewed Nov 22, 2022

View reviewed changes

development/cpp/configuring_an_ide/visual_studio_code.rst Show resolved Hide resolved

NeroBurner force-pushed the tools_target_unification branch from 79f1267 to 4e97306 Compare November 22, 2022 20:05

Maran23 reviewed Nov 23, 2022

View reviewed changes

development/cpp/configuring_an_ide/visual_studio_code.rst Show resolved Hide resolved

Calinou mentioned this pull request Nov 23, 2022

Documentation pages to add or update for Godot 4.0 #5121

Open

41 tasks

raulsntos reviewed Nov 24, 2022

View reviewed changes

Maran23 reviewed Dec 1, 2022

View reviewed changes

development/compiling/introduction_to_the_buildsystem.rst Outdated Show resolved Hide resolved

Maran23 reviewed Dec 1, 2022

View reviewed changes

development/cpp/configuring_an_ide/visual_studio_code.rst Outdated Show resolved Hide resolved

Maran23 reviewed Dec 1, 2022

View reviewed changes

development/compiling/introduction_to_the_buildsystem.rst Outdated Show resolved Hide resolved

Maran23 reviewed Dec 1, 2022

View reviewed changes

development/compiling/compiling_for_macos.rst Outdated Show resolved Hide resolved

Maran23 approved these changes Dec 6, 2022

View reviewed changes

NeroBurner added 11 commits December 6, 2022 20:56

Replace 'bits=64/32' with 'arch=x86_64/x86_32'

4579107

compiling_with_mono: replace 'bits' with 'arch'

98921b4

setup_IDE: explicitly mention 'dev_build=yes' and update binary names

0bb0509

Changed mentions for `vscode`, `kdevelop` and `qtcreator`

linuxbsd: mention production=yes flag

ae74caa

comp_android: replace android_arch with just arch

988f065

linuxbsd: no server platform, use --headless command instead

506ee88

macos: platrom=server replaced with --headless command line

3ed827f

mono: remove mono_glue=yes/no, replace with build_assemblies.py call

3368730

update vscode_3_tasks.json.png

f8ffbf2

Update vscode_2_launch.json.png with current example

5c63dcb

NeroBurner and others added 2 commits December 6, 2022 20:56

docs: apply typo fixes from code review

6d6ccad

Co-authored-by: Raul Santos <[email protected]> Co-authored-by: Marius Hanl <[email protected]>

apply suggestions by @Maran23

fe9b6fd

NeroBurner force-pushed the tools_target_unification branch from c08d4c5 to fe9b6fd Compare December 6, 2022 19:57

Maran23 approved these changes Dec 7, 2022

View reviewed changes

clayjohn reviewed Dec 7, 2022

View reviewed changes

Apply suggestions from code review by @bruvzg

b648b72

Co-authored-by: Clay John <[email protected]>

clayjohn approved these changes Dec 7, 2022

View reviewed changes

Maran23 approved these changes Dec 7, 2022

View reviewed changes

clayjohn merged commit 66679c7 into godotengine:master Dec 7, 2022

NeroBurner deleted the tools_target_unification branch December 12, 2022 07:12

YuriSizov mentioned this pull request Dec 12, 2022

Fix Introduction to the buildsystem links in CLI documentation #6483

Merged

YuriSizov mentioned this pull request Dec 24, 2022

Fix formatting issues from recent PRs #6511

Merged

YuriSizov mentioned this pull request Jan 3, 2023

New 4.0 scons build options #6274

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document the Unification of tools/target build type configuration #6316

Document the Unification of tools/target build type configuration #6316

NeroBurner commented Oct 15, 2022

bruvzg Oct 15, 2022

NeroBurner Oct 16, 2022

gamedevsam left a comment

NeroBurner commented Oct 16, 2022

NeroBurner commented Nov 22, 2022

raulsntos left a comment

raulsntos Nov 22, 2022

raulsntos Nov 22, 2022

raulsntos Nov 22, 2022

raulsntos Nov 22, 2022

raulsntos Nov 22, 2022

Calinou Dec 2, 2022 •

edited

Loading

raulsntos Nov 24, 2022

Maran23 commented Dec 1, 2022

Maran23 commented Dec 1, 2022

Calinou commented Dec 1, 2022

Maran23 left a comment

NeroBurner commented Dec 6, 2022

Maran23 left a comment

clayjohn left a comment

clayjohn left a comment

Maran23 left a comment

clayjohn commented Dec 7, 2022

Anutrix commented Jan 3, 2023


		scons p=<platform> tools=yes module_mono_enabled=yes mono_glue=no
		scons p=<platform> target=editor module_mono_enabled=yes mono_glue=no

Document the Unification of tools/target build type configuration #6316

Document the Unification of tools/target build type configuration #6316

Conversation

NeroBurner commented Oct 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gamedevsam left a comment

Choose a reason for hiding this comment

NeroBurner commented Oct 16, 2022

NeroBurner commented Nov 22, 2022

raulsntos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Calinou Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maran23 commented Dec 1, 2022

Maran23 commented Dec 1, 2022

Calinou commented Dec 1, 2022

Maran23 left a comment

Choose a reason for hiding this comment

NeroBurner commented Dec 6, 2022

Maran23 left a comment

Choose a reason for hiding this comment

clayjohn left a comment

Choose a reason for hiding this comment

clayjohn left a comment

Choose a reason for hiding this comment

Maran23 left a comment

Choose a reason for hiding this comment

clayjohn commented Dec 7, 2022

Anutrix commented Jan 3, 2023

Calinou Dec 2, 2022 •

edited

Loading