mirror of
https://github.com/godotengine/godot-contributing-docs.git
synced 2025-12-31 05:48:13 +03:00
Add content from docs repository.
Re-structure according to actual content. WIP, some references are still broken.
This commit is contained in:
@@ -1,2 +0,0 @@
|
||||
Code Style
|
||||
==========
|
||||
@@ -1,4 +0,0 @@
|
||||
Creating Pull Requests
|
||||
======================
|
||||
|
||||
TODO
|
||||
@@ -1,4 +0,0 @@
|
||||
Documentation
|
||||
=============
|
||||
|
||||
TODO
|
||||
@@ -1,4 +0,0 @@
|
||||
Getting Started
|
||||
===============
|
||||
|
||||
TODO
|
||||
@@ -1,4 +0,0 @@
|
||||
Reviewing Pull Requests
|
||||
=======================
|
||||
|
||||
TODO
|
||||
@@ -1,4 +0,0 @@
|
||||
Unit Testing
|
||||
============
|
||||
|
||||
TODO
|
||||
19
documentation/first_steps.rst
Normal file
19
documentation/first_steps.rst
Normal file
@@ -0,0 +1,19 @@
|
||||
Getting started
|
||||
===============
|
||||
|
||||
There are two separate resources referred to as "documentation" in Godot:
|
||||
|
||||
- **The class reference.** This is the documentation for the complete Godot API
|
||||
as exposed to GDScript and the other scripting languages. It can be consulted
|
||||
offline, directly in Godot's code editor, or online at Godot :ref:`Class Reference
|
||||
<doc_class_reference>`. To contribute to the class reference, you have to edit the
|
||||
XML file corresponding to the class and make a pull request.
|
||||
See :ref:`doc_updating_the_class_reference` and :ref:`doc_class_reference_primer`
|
||||
for more details.
|
||||
|
||||
- **The tutorials and engine documentation and its translations.**
|
||||
This is the part you are reading now, which is distributed in the HTML format.
|
||||
Its contents are generated from plain text files in the reStructured Text
|
||||
(rst) format, to which you can contribute via pull requests on the
|
||||
`godot-docs <https://github.com/godotengine/godot-docs>`_ GitHub repository.
|
||||
See :ref:`doc_contributing_to_the_documentation` for more details.
|
||||
422
documentation/translation.rst
Normal file
422
documentation/translation.rst
Normal file
@@ -0,0 +1,422 @@
|
||||
.. _doc_editor_and_docs_localization:
|
||||
|
||||
Translation
|
||||
===========
|
||||
|
||||
.. highlight:: none
|
||||
|
||||
Godot aims to make game development available to everyone, including people who
|
||||
may not know or be comfortable with English. Therefore, we do our best to make
|
||||
the most important resources available in many languages, thanks to the
|
||||
translation effort of the community.
|
||||
|
||||
These resources include:
|
||||
|
||||
1. The `Godot editor's interface <https://hosted.weblate.org/projects/godot-engine/godot/>`__.
|
||||
2. The `class reference <https://hosted.weblate.org/projects/godot-engine/godot-class-reference/>`__,
|
||||
available both online and in the editor.
|
||||
3. The `online documentation <https://hosted.weblate.org/projects/godot-engine/godot-docs/>`__
|
||||
(editor manual and tutorials).
|
||||
|
||||
To manage translations, we use the GNU gettext file format (``PO`` files), and
|
||||
the open source `Weblate <https://weblate.org>`__ web-based localization
|
||||
platform, which allows easy collaboration of many contributors to complete the
|
||||
translation for the various components, and keep them up to date. Click the bold
|
||||
links above to access each resource on Weblate.
|
||||
|
||||
This page gives an overview of the general translation workflow on Weblate, and
|
||||
some resource-specific instructions on e.g. how to handle some keywords or the
|
||||
localization of images.
|
||||
|
||||
.. tip::
|
||||
|
||||
Translating all the official Godot content is a massive undertaking, so we
|
||||
advise prioritizing the resources as they are listed above: first the editor
|
||||
interface, then the class reference, then the online documentation.
|
||||
|
||||
Using Weblate for translations
|
||||
------------------------------
|
||||
|
||||
While our translations eventually reside in the Git repositories of the Godot
|
||||
engine and its documentation, all translation updates are handled through
|
||||
Weblate, and thus direct pull requests to the Git repositories are not accepted.
|
||||
Translations are synced manually between Weblate and the Godot repositories by
|
||||
maintainers.
|
||||
|
||||
You should therefore `register on Weblate <https://hosted.weblate.org/accounts/register/>`__
|
||||
to contribute to Godot's translations.
|
||||
|
||||
Once signed in, browse to the Godot resource which you want to contribute to (in
|
||||
this page we will use the `editor translation <https://hosted.weblate.org/projects/godot-engine/godot/>`__
|
||||
as an example) to find the list of all languages:
|
||||
|
||||
.. image:: img/l10n_01_language_list.png
|
||||
|
||||
.. seealso::
|
||||
|
||||
Feel free to consult Weblate's own documentation on the `translation
|
||||
workflow <https://docs.weblate.org/en/latest/user/translating.html>`__ for
|
||||
more details.
|
||||
|
||||
Adding a new language
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
If your language is already listed, click on its name to access the overview,
|
||||
and skip the rest of this section.
|
||||
|
||||
If your language is not listed, scroll to the bottom of the list of languages
|
||||
and click the "Start new translation" button, and select the language you want
|
||||
to translate to:
|
||||
|
||||
.. image:: img/l10n_02_new_translation.png
|
||||
|
||||
.. important::
|
||||
|
||||
If your language is spoken in several countries with only limited regional
|
||||
variations, please consider adding it with its generic variant (e.g. ``fr``
|
||||
for French) instead of a regional variant (e.g. ``fr_FR`` for French
|
||||
(France), ``fr_CA`` for French (Canada), or ``fr_DZ`` for French (Algeria)).
|
||||
|
||||
Godot has a huge amount of content to translate, so duplicating the work for
|
||||
regional variants should only be done if the language variations are
|
||||
significant enough. Additionally, if a translation is done with for a
|
||||
regional variant, it will only be available automatically for users located
|
||||
in this region (or having their system language configured for this region).
|
||||
|
||||
When regional variations are significant enough to warrant separate
|
||||
translations, we advise to focus on completing a generic variant first if
|
||||
possible, then duplicate the fully completed translation for regional
|
||||
variants and do the relevant edits. This is typically a good strategy for
|
||||
e.g. Spanish (work on ``es`` first, then duplicate it to ``es_AR``,
|
||||
``es_ES``, ``es_MX``, etc. if necessary) or Portuguese (``pt_BR`` vs
|
||||
``pt_PT``).
|
||||
|
||||
Translation interface
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Once a language has been selected, you will see an overview of the translation
|
||||
status, including how many strings are left to translate or review. Each item
|
||||
can be clicked and used to browse through the corresponding list. You can also
|
||||
click the "Translate" button to get started on the list of strings needing
|
||||
action.
|
||||
|
||||
.. image:: img/l10n_03_translation_overview.png
|
||||
|
||||
After selecting a list of clicking "Translate", you will see the main
|
||||
translation interface where all the work happens:
|
||||
|
||||
.. image:: img/l10n_04_translation_interface.png
|
||||
|
||||
On that page, you have:
|
||||
|
||||
- A toolbar which lets you cycle through strings of the current list, change
|
||||
to another predefined list or do a custom search, etc. There is also a "Zen"
|
||||
editing mode with a simplified interface.
|
||||
- The actual string you are working on in the "Translation" panel. By default,
|
||||
there should be the English source string and an edit box for your language.
|
||||
If you are familiar with other languages, you can add them in your user
|
||||
settings to give you more context for translation.
|
||||
Once you are done editing the current string, press "Save" to confirm changes
|
||||
and move to the next entry. Alternatively, use the "Skip" button to skip it.
|
||||
The "Needs editing" checkbox means that the original string was updated, and
|
||||
the translation therefore needs review to take those changes into account (in
|
||||
PO jargon, these are so-called "fuzzy" strings). Such strings won't be used
|
||||
in the translation until fixed.
|
||||
- The bottom panel has various tools which can help with the translation
|
||||
effort, such as context from nearby strings (usually from the same editor
|
||||
tool or documentation page, so they might use similar terms), comments from
|
||||
other translators, machine translations, and a list of all other existing
|
||||
translations for that string.
|
||||
- On the top right, the glossary shows terms for which an entry has been added
|
||||
previously, and which are included in the current string. For example, if
|
||||
you decided with fellow translators to use a specific translation for the
|
||||
"node" term in Godot, you can add it to the glossary to ensure that other
|
||||
translators use the same convention.
|
||||
- The bottom right panel includes information on the source string. The most
|
||||
relevant item is the "source string location", which links you to the
|
||||
original string on GitHub. You may need to search for the string in the page
|
||||
to locate it and its surrounding context.
|
||||
|
||||
Locating original content
|
||||
-------------------------
|
||||
|
||||
PO files are an ordered list of source strings (``msgid``) and their translation
|
||||
(``msgstr``), and by default, Weblate will present the strings in that order. It
|
||||
can therefore be useful to understand how the content is organized in the PO
|
||||
files to help you locate the original content and use it as a reference when
|
||||
translating.
|
||||
|
||||
.. important::
|
||||
|
||||
It is primordial to use the original context as reference when translating,
|
||||
as many words have several possible translations depending on the context.
|
||||
Using the wrong translation can actually be detrimental to the user and make
|
||||
things harder to understand than if they stayed in English.
|
||||
Using the context also makes the translation effort much easier and more
|
||||
enjoyable, as you can see directly if the translation you wrote will make
|
||||
sense in context.
|
||||
|
||||
- The editor interface's translation template is generated by parsing all the
|
||||
C++ source code in **alphabetical order**, so all the strings defined in a
|
||||
given file will be grouped together. For example, if the "source string
|
||||
location" indicates ``editor/code_editor.cpp``, the current string (and the
|
||||
nearby ones) is defined in the ``editor/code_editor.cpp`` code file, and is
|
||||
thereby related to the code editors in Godot (GDScript, shaders).
|
||||
- The online documentation's translation template is generated from the source
|
||||
RST files in the same order as seen in the **table of contents**, so for
|
||||
example the first strings are from the front page of the documentation.
|
||||
The recommended workflow is therefore to find a unique string corresponding to
|
||||
a page that you want to translate, and then translate all the strings with the
|
||||
same source string location while comparing with the online version of that
|
||||
page in English. An example of source string location could be
|
||||
``getting_started/step_by_step/nodes_and_scenes.rst`` for the
|
||||
page :ref:`doc_nodes_and_scenes`.
|
||||
- The class reference's translation template is generated from the source XML
|
||||
files in **alphabetical order**, which is also the same as the order of the
|
||||
table of contents for the online version. You can therefore locate the source
|
||||
string corresponding to the brief description of a given class to find the
|
||||
first string to translate and all other descriptions from that class should be
|
||||
in the subsequent strings on Weblate. For example, the descriptions for the
|
||||
:ref:`class_Node2D` class would have the source string location
|
||||
``doc/classes/Node2D.xml``.
|
||||
|
||||
A handy tool to locate specific pages/classes is to use Weblate's advanced
|
||||
search feature, and especially the "Location strings" query (which can also be
|
||||
used with the ``location:`` token, e.g. ``location:nodes_and_scenes.rst``):
|
||||
|
||||
.. image:: img/l10n_05_search_location.png
|
||||
|
||||
.. image:: img/l10n_06_browse_by_location.png
|
||||
|
||||
.. note::
|
||||
|
||||
When a given source string is used in multiple source locations, they will
|
||||
all be concatenated into one. For example, the above
|
||||
``location:nodes_and_scenes.rst`` query would land first on the
|
||||
"Introduction" source string which is used in dozens of pages, including
|
||||
some that come before ``nodes_and_scenes.rst`` in the template. Clicking the
|
||||
"Next" button then brings us to the "Scene and nodes" title string displayed
|
||||
above.
|
||||
So it may happen that a given paragraph or section title is not at the
|
||||
location you'd expect it when reading the online version of a page.
|
||||
|
||||
Respecting the markup syntax
|
||||
----------------------------
|
||||
|
||||
Each translation resource originates from a different source code format, and
|
||||
having some notions on the markup language used for each resource is important
|
||||
to avoid creating syntax errors in your translations.
|
||||
|
||||
Editor interface (C++)
|
||||
~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
The editor translations originate from C++ strings, and may use:
|
||||
|
||||
- **C format specifiers** such as ``%s`` (a string) or ``%d`` (a number). These
|
||||
specifiers are replaced by content at runtime, and should be preserved and
|
||||
placed in your translation where necessary for it to be meaningful after
|
||||
substitution. You may need to refer to the source string location to
|
||||
understand what kind of content will be substituted if it's not clear from the
|
||||
sentence. Example (``%s`` will be substituted with a file name or path):
|
||||
|
||||
::
|
||||
|
||||
# PO file:
|
||||
"There is no '%s' file."
|
||||
|
||||
# Weblate:
|
||||
There is no '%s' file.
|
||||
|
||||
- **C escape characters** such as ``\n`` (line break) or ``\t`` (tabulation). In
|
||||
the Weblate editor, the ``\n`` characters are replaced by ``↵`` (return) and
|
||||
``\t`` by ``↹``. Tabs are not used much, but you should make sure to use line
|
||||
breaks in the same way as the original English string (Weblate will issue a
|
||||
warning if you don't). Line breaks might sometimes be used for vertical
|
||||
spacing, or manual wrapping of long lines which would otherwise be too long
|
||||
especially in the editor translation). Example:
|
||||
|
||||
::
|
||||
|
||||
# PO file:
|
||||
"Scene '%s' is currently being edited.\n"
|
||||
"Changes will only take effect when reloaded."
|
||||
|
||||
# Weblate:
|
||||
Scene '%s' is currently being edited.↵
|
||||
Changes will only take effect when reloaded.
|
||||
|
||||
.. note::
|
||||
Only logical order of the characters matters, in the right-to-left text, format
|
||||
specifiers may be displayed as ``s%``.
|
||||
|
||||
Online documentation (RST)
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
The documentation translations originate from reStructuredText (RST) files,
|
||||
which also use their own markup syntax to style text, create internal and
|
||||
external links, etc. Here are some examples:
|
||||
|
||||
::
|
||||
|
||||
# "development" is styled bold.
|
||||
# "Have a look here" is a link pointing to https://docs.godotengine.org/en/latest.
|
||||
# You should translate "Have a look here", but not the URL, unless there is
|
||||
# a matching URL for the same content in your language.
|
||||
# Note: The `, <, >, and _ characters all have a meaning in the hyperlink
|
||||
# syntax and should be preserved.
|
||||
|
||||
Looking for the documentation of the current **development** branch?
|
||||
`Have a look here <https://docs.godotengine.org/en/latest>`_.
|
||||
|
||||
# "|supported|" is an inline reference to an image and should stay unchanged.
|
||||
# "master" uses the markup for inline code, and will be styled as such.
|
||||
# Note: Inline code in RST uses 2 backticks on each side, unlike Markdown.
|
||||
# Single backticks are used for hyperlinks.
|
||||
|
||||
|supported| Backwards-compatible new features (backported from the ``master``
|
||||
branch) as well as bug, security, and platform support fixes.
|
||||
|
||||
# The :ref: Sphinx "role" is used for internal references to other pages of
|
||||
# the documentation.
|
||||
# It can be used with only the reference name of a page (which should not be
|
||||
# changed), in which case the title of that page will be displayed:
|
||||
|
||||
See :ref:`doc_ways_to_contribute`.
|
||||
|
||||
# Or it can be used with an optional custom title, which should thus be translated:
|
||||
|
||||
See :ref:`how to contribute <doc_ways_to_contribute>`.
|
||||
|
||||
# You may encounter other Sphinx roles, such as :kbd: used for shortcut keys.
|
||||
# You can translate the content between backticks to match the usual key names,
|
||||
# if it's different from the English one.
|
||||
|
||||
Save the scene. Click Scene -> Save, or press :kbd:`Ctrl + S` on Windows/Linux
|
||||
or :kbd:`Cmd + S` on macOS.
|
||||
|
||||
.. seealso::
|
||||
|
||||
See Sphinx's `reStructured Text primer <https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`__
|
||||
for a quick overview of the markup language you may find in source strings.
|
||||
You may encounter especially the inline markup (bold, italics, inline code)
|
||||
and the internal and external hyperlink markup.
|
||||
|
||||
Class reference (BBCode)
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
The class reference is documented in the main Godot repository using XML files,
|
||||
and with BBCode-like markup for styling and internal references.
|
||||
|
||||
Some of the tags used are from the original BBCode (e.g. ``[b]Bold[/b]`` and
|
||||
``[i]Italics[/i]``), while others are Godot-specific and used for advanced
|
||||
features such as inline code (e.g. ``[code]true[/code]``), linking to another
|
||||
class (e.g. ``[Node2D]``) or to a property in a given class (e.g.
|
||||
``[member Node2D.position]``), or for multiline code blocks. Example:
|
||||
|
||||
::
|
||||
|
||||
Returns a color according to the standardized [code]name[/code] with [code]alpha[/code] ranging from 0 to 1.
|
||||
[codeblock]
|
||||
red = ColorN("red", 1)
|
||||
[/codeblock]
|
||||
Supported color names are the same as the constants defined in [Color].
|
||||
|
||||
In the above example, ``[code]name[/code]``, ``[code]alpha[/code]``, and
|
||||
``[Color]`` should *not* be translated, as they refer respectively to argument
|
||||
names and a class of the Godot API. Similarly, the contents of the
|
||||
``[codeblock]`` should not be translated, as ``ColorN`` is a function of the
|
||||
Godot API and ``"red"`` is one of the named colors it supports. At most, you can
|
||||
translate the name of the variable which holds the result (``red = ...``).
|
||||
|
||||
Note also that in the XML, each line is a paragraph, so you should not add line
|
||||
breaks if they are not part of the original translation.
|
||||
|
||||
.. seealso::
|
||||
|
||||
See our documentation for class reference writers for the :ref:`list of
|
||||
BBCode-like tags <doc_class_reference_bbcode>` which are used
|
||||
throughout the class reference.
|
||||
|
||||
Offline translation and testing
|
||||
-------------------------------
|
||||
|
||||
While we advise using the Weblate interface to write translations, you also have
|
||||
the possibility to download the PO file locally to translate it with your
|
||||
preferred PO editing application, such as `Poedit <https://poedit.net/>`__ or
|
||||
`Lokalize <https://userbase.kde.org/Lokalize>`__.
|
||||
|
||||
To download the PO file locally, browse to the translation overview for your
|
||||
language, and select the first item in the "Files" menu:
|
||||
|
||||
.. image:: img/l10n_07_download_po_file.png
|
||||
|
||||
Once you are done with a series of edits, use the "Upload translation" item in
|
||||
that same menu and select your file. Choose "Add as translation" for the file
|
||||
upload mode.
|
||||
|
||||
.. note::
|
||||
|
||||
If a significant amount of time has passed between your download of the PO
|
||||
file and the upload of the edited version, there is a risk to overwrite the
|
||||
translations authored by other contributors in the meantime. This is why we
|
||||
advise to use the online interface so that you always work on the latest
|
||||
version.
|
||||
|
||||
If you want to test changes locally (especially for the editor translation), you
|
||||
can use the downloaded PO file and :ref:`compile Godot from source <toc-devel-compiling>`.
|
||||
|
||||
Rename the editor translation PO file to ``<lang>.po`` (e.g. ``eo.po`` for
|
||||
Esperanto) and place it in the ``editor/translations/`` folder
|
||||
(`GitHub <https://github.com/godotengine/godot/tree/master/editor/translations>`__).
|
||||
|
||||
You can also test class reference changes the same way by renaming the PO file
|
||||
similarly and placing it in the ``doc/translations/`` folder
|
||||
(`GitHub <https://github.com/godotengine/godot/tree/master/doc/translations>`__).
|
||||
|
||||
Localizing documentation images
|
||||
-------------------------------
|
||||
|
||||
The online documentation includes many images, which can be screenshots of the
|
||||
Godot editor, custom-made graphs, of any other kind of visual content. Some of
|
||||
it includes text and might thus be relevant to localize in your language.
|
||||
|
||||
This part is not handled via Weblate, but directly on the `godot-docs-l10n
|
||||
<https://github.com/godotengine/godot-docs-l10n>`_ Git repository where the
|
||||
documentation translations are synced from Weblate.
|
||||
|
||||
.. note::
|
||||
|
||||
The workflow is not the most straightforward and requires some knowledge of
|
||||
Git. We plan to work on a simplified Web tool which could be used to manage
|
||||
image localization in a convenient way, abstracting away these steps.
|
||||
|
||||
To translate an image, you should first locate it in the original English
|
||||
documentation. To do so, browse the relevant page in the docs, e.g.
|
||||
:ref:`doc_intro_to_the_editor_interface`. Click the "Edit on GitHub" link in the
|
||||
top right corner:
|
||||
|
||||
.. image:: img/l10n_08_edit_on_github.png
|
||||
|
||||
On GitHub, click on the image you want to translate. If relevant, click on
|
||||
"Download" to download it locally and edit it with an image edition tool.
|
||||
Note the full path to the image as it will be needed further down (here
|
||||
``getting_started/step_by_step/img/project_manager_first_open.png``).
|
||||
|
||||
.. image:: img/l10n_09_path_to_image.png
|
||||
|
||||
Create your localized version of the image, either by editing the English one,
|
||||
or by taking a screenshot of the editor with your language, if it's an editor
|
||||
screenshot. Some images may also have source files available in SVG format, so
|
||||
you can browse the ``img/`` folder which contains them to check for that.
|
||||
|
||||
Name your localized image like the original one, but with the language code
|
||||
added before the extension, e.g. ``project_manager_first_open.png`` would become
|
||||
``project_manager_first_open.fr.png`` for the French localization.
|
||||
|
||||
Finally, on godot-docs-l10n_, recreate the same folder structure as for the
|
||||
original image in the ``images`` subfolder
|
||||
(`GitHub <https://github.com/godotengine/godot-docs-l10n/tree/master/images>`_),
|
||||
and place your translated image there. In our example, the end result should be
|
||||
``images/getting_started/step_by_step/img/project_manager_first_open.fr.png``.
|
||||
|
||||
Repeat this for other images and :ref:`make a Pull Request <doc_pr_workflow>`.
|
||||
393
engine/code_style.rst
Normal file
393
engine/code_style.rst
Normal file
@@ -0,0 +1,393 @@
|
||||
.. _doc_code_style_guidelines:
|
||||
|
||||
Code style guidelines
|
||||
=====================
|
||||
|
||||
.. highlight:: shell
|
||||
|
||||
When contributing to Godot's source code, you will be expected to follow the
|
||||
style guidelines outlined below. Some of them are checked via the Continuous
|
||||
Integration process and reviewers will ask you to fix potential issues, so
|
||||
best setup your system as outlined below to ensure all your commits follow the
|
||||
guidelines.
|
||||
|
||||
C++ and Objective-C
|
||||
-------------------
|
||||
|
||||
There are no written guidelines, but the code style agreed upon by the
|
||||
developers is enforced via the `clang-format <https://clang.llvm.org/docs/ClangFormat.html>`__
|
||||
code beautifier, which takes care for you of all our conventions.
|
||||
To name a few:
|
||||
|
||||
- Indentation and alignment are both tab based (respectively one and two tabs)
|
||||
- One space around math and assignments operators as well as after commas
|
||||
- Pointer and reference operators are affixed to the variable identifier, not
|
||||
to the type name
|
||||
- See further down regarding header includes
|
||||
|
||||
The rules used by clang-format are outlined in the
|
||||
`.clang-format <https://github.com/godotengine/godot/blob/master/.clang-format>`__
|
||||
file of the Godot repository.
|
||||
|
||||
As long as you ensure that your style matches the surrounding code and that you're
|
||||
not introducing trailing whitespace or space-based indentation, you should be
|
||||
fine. If you plan to contribute regularly, however, we strongly advise that you
|
||||
set up clang-format locally to check and automatically fix all your commits.
|
||||
|
||||
.. warning:: Godot's code style should *not* be applied to third-party code,
|
||||
i.e. code that is included in Godot's source tree, but was not written
|
||||
specifically for our project. Such code usually comes from
|
||||
different upstream projects with their own style guides (or lack
|
||||
thereof), and don't want to introduce differences that would make
|
||||
syncing with upstream repositories harder.
|
||||
|
||||
Third-party code is usually included in the ``thirdparty/`` folder
|
||||
and can thus easily be excluded from formatting scripts. For the
|
||||
rare cases where a third-party code snippet needs to be included
|
||||
directly within a Godot file, you can use
|
||||
``/* clang-format off */`` and ``/* clang-format on */`` to tell
|
||||
clang-format to ignore a chunk of code.
|
||||
|
||||
.. seealso::
|
||||
|
||||
These guidelines only cover code formatting. See :ref:`doc_cpp_usage_guidelines`
|
||||
for a list of language features that are permitted in pull requests.
|
||||
|
||||
|
||||
Using clang-format locally
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
You need to use **clang-format 17** to be compatible with Godot's format. Later versions might
|
||||
be suitable, but previous versions may not support all used options, or format
|
||||
some things differently, leading to style issues in pull requests.
|
||||
|
||||
.. _doc_code_style_guidelines_pre_commit_hook:
|
||||
|
||||
Pre-commit hook
|
||||
^^^^^^^^^^^^^^^
|
||||
|
||||
For ease of use, we provide hooks for Git with the `pre-commit <https://pre-commit.com/>`__
|
||||
Python framework that will run clang-format automatically on all your commits with the
|
||||
correct version of clang-format.
|
||||
To set up:
|
||||
|
||||
::
|
||||
|
||||
pip install pre-commit
|
||||
pre-commit install
|
||||
|
||||
|
||||
You can also run the hook manually with ``pre-commit run``.
|
||||
|
||||
.. note::
|
||||
|
||||
Previously, we supplied a hook in the folder ``misc/hooks``. If you copied the
|
||||
script manually, these hooks should still work, but symlinks will be broken.
|
||||
If you are using the new system, run ``rm .git/hooks/*`` to remove the old hooks
|
||||
that are no longer needed.
|
||||
|
||||
|
||||
Installation
|
||||
^^^^^^^^^^^^
|
||||
|
||||
Here's how to install clang-format:
|
||||
|
||||
- Linux: It will usually be available out-of-the-box with the clang toolchain
|
||||
packaged by your distribution. If your distro version is not the required one,
|
||||
you can download a pre-compiled version from the
|
||||
`LLVM website <https://releases.llvm.org/download.html>`__, or if you are on
|
||||
a Debian derivative, use the `upstream repos <https://apt.llvm.org/>`__.
|
||||
- macOS and Windows: You can download precompiled binaries from the
|
||||
`LLVM website <https://releases.llvm.org/download.html>`__. You may need to add
|
||||
the path to the binary's folder to your system's ``PATH`` environment
|
||||
variable to be able to call clang-format out of the box.
|
||||
|
||||
You then have different possibilities to apply clang-format to your changes:
|
||||
|
||||
Manual usage
|
||||
^^^^^^^^^^^^
|
||||
|
||||
You can apply clang-format manually for one or more files with the following
|
||||
command:
|
||||
|
||||
::
|
||||
|
||||
clang-format -i <path/to/file(s)>
|
||||
|
||||
- ``-i`` means that the changes should be written directly to the file (by
|
||||
default clang-format would only output the fixed version to the terminal).
|
||||
- The path can point to several files, either one after the other or using
|
||||
wildcards like in a typical Unix shell. Be careful when globbing so that
|
||||
you don't run clang-format on compiled objects (.o and .a files) that are
|
||||
in Godot's tree. So better use ``core/*.{cpp,h}`` than ``core/*``.
|
||||
|
||||
|
||||
IDE plugin
|
||||
^^^^^^^^^^
|
||||
|
||||
Most IDEs or code editors have beautifier plugins that can be configured to run
|
||||
clang-format automatically, for example, each time you save a file.
|
||||
|
||||
Here is a non-exhaustive list of beautifier plugins for some IDEs:
|
||||
|
||||
- Qt Creator: `Beautifier plugin <https://doc.qt.io/qtcreator/creator-beautifier.html>`__
|
||||
- Visual Studio Code: `Clang-Format <https://marketplace.visualstudio.com/items?itemName=xaver.clang-format>`__
|
||||
- Visual Studio: `Clang Power Tools 2022 <https://marketplace.visualstudio.com/items?itemName=caphyon.ClangPowerTools2022>`__
|
||||
- vim: `vim-clang-format <https://github.com/rhysd/vim-clang-format>`__
|
||||
- CLion: Starting from version ``2019.1``, no plugin is required. Instead, enable
|
||||
`ClangFormat <https://www.jetbrains.com/help/clion/clangformat-as-alternative-formatter.html#clion-support>`__
|
||||
|
||||
(Pull requests are welcome to extend this list with tested plugins.)
|
||||
|
||||
.. _doc_code_style_guidelines_header_includes:
|
||||
|
||||
Header includes
|
||||
~~~~~~~~~~~~~~~
|
||||
|
||||
When adding new C++ or Objective-C files or including new headers in existing
|
||||
ones, the following rules should be followed:
|
||||
|
||||
- The first lines in the file should be Godot's copyright header and MIT
|
||||
license, copy-pasted from another file. Make sure to adjust the filename.
|
||||
- In a ``.h`` header, include guards should be used with the form
|
||||
``FILENAME_H``.
|
||||
|
||||
- In a ``.cpp`` file (e.g. ``filename.cpp``), the first include should be the
|
||||
one where the class is declared (e.g. ``#include "filename.h"``), followed by
|
||||
an empty line for separation.
|
||||
- Then come headers from Godot's own code base, included in alphabetical order
|
||||
(enforced by ``clang-format``) with paths relative to the root folder. Those
|
||||
includes should be done with quotes, e.g. ``#include "core/object.h"``. The
|
||||
block of Godot header includes should then be followed by an empty line for
|
||||
separation.
|
||||
- Finally, third-party headers (either from ``thirdparty`` or from the system's
|
||||
include paths) come next and should be included with the < and > symbols, e.g.
|
||||
``#include <png.h>``. The block of third-party headers should also be followed
|
||||
by an empty line for separation.
|
||||
- Godot and third-party headers should be included in the file that requires
|
||||
them, i.e. in the `.h` header if used in the declarative code or in the `.cpp`
|
||||
if used only in the imperative code.
|
||||
|
||||
Example:
|
||||
|
||||
.. code-block:: cpp
|
||||
:caption: my_new_file.h
|
||||
|
||||
/**************************************************************************/
|
||||
/* my_new_file.h */
|
||||
/**************************************************************************/
|
||||
/* This file is part of: */
|
||||
/* GODOT ENGINE */
|
||||
/* https://godotengine.org */
|
||||
/**************************************************************************/
|
||||
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
|
||||
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
|
||||
/* */
|
||||
/* Permission is hereby granted, free of charge, to any person obtaining */
|
||||
/* a copy of this software and associated documentation files (the */
|
||||
/* "Software"), to deal in the Software without restriction, including */
|
||||
/* without limitation the rights to use, copy, modify, merge, publish, */
|
||||
/* distribute, sublicense, and/or sell copies of the Software, and to */
|
||||
/* permit persons to whom the Software is furnished to do so, subject to */
|
||||
/* the following conditions: */
|
||||
/* */
|
||||
/* The above copyright notice and this permission notice shall be */
|
||||
/* included in all copies or substantial portions of the Software. */
|
||||
/* */
|
||||
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
|
||||
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
|
||||
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
|
||||
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
|
||||
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
|
||||
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
|
||||
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
|
||||
/**************************************************************************/
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "core/hash_map.h"
|
||||
#include "core/list.h"
|
||||
#include "scene/gui/control.h"
|
||||
|
||||
#include <png.h>
|
||||
|
||||
.. code-block:: cpp
|
||||
:caption: my_new_file.cpp
|
||||
|
||||
/**************************************************************************/
|
||||
/* my_new_file.cpp */
|
||||
/**************************************************************************/
|
||||
/* This file is part of: */
|
||||
/* GODOT ENGINE */
|
||||
/* https://godotengine.org */
|
||||
/**************************************************************************/
|
||||
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
|
||||
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
|
||||
/* */
|
||||
/* Permission is hereby granted, free of charge, to any person obtaining */
|
||||
/* a copy of this software and associated documentation files (the */
|
||||
/* "Software"), to deal in the Software without restriction, including */
|
||||
/* without limitation the rights to use, copy, modify, merge, publish, */
|
||||
/* distribute, sublicense, and/or sell copies of the Software, and to */
|
||||
/* permit persons to whom the Software is furnished to do so, subject to */
|
||||
/* the following conditions: */
|
||||
/* */
|
||||
/* The above copyright notice and this permission notice shall be */
|
||||
/* included in all copies or substantial portions of the Software. */
|
||||
/* */
|
||||
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
|
||||
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
|
||||
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
|
||||
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
|
||||
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
|
||||
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
|
||||
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
|
||||
/**************************************************************************/
|
||||
|
||||
#include "my_new_file.h"
|
||||
|
||||
#include "core/math/math_funcs.h"
|
||||
#include "scene/gui/line_edit.h"
|
||||
|
||||
#include <zlib.h>
|
||||
#include <zstd.h>
|
||||
|
||||
Java
|
||||
----
|
||||
|
||||
Godot's Java code (mostly in ``platform/android``) is also enforced via
|
||||
``clang-format``, so see the instructions above to set it up. Keep in mind that
|
||||
this style guide only applies to code written and maintained by Godot, not
|
||||
third-party code such as the ``java/src/com/google`` subfolder.
|
||||
|
||||
Python
|
||||
------
|
||||
|
||||
Godot's SCons buildsystem is written in Python, and various scripts included
|
||||
in the source tree are also using Python.
|
||||
|
||||
For those, we use the `Ruff linter and code formatter <https://docs.astral.sh/ruff/>`__.
|
||||
|
||||
Using ruff locally
|
||||
~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
First of all, you will need to install Ruff. Ruff requires Python 3.7+ to run.
|
||||
|
||||
Installation
|
||||
^^^^^^^^^^^^
|
||||
|
||||
Here's how to install ruff:
|
||||
|
||||
::
|
||||
|
||||
pip3 install ruff --user
|
||||
|
||||
|
||||
You then have different possibilities to apply ruff to your changes:
|
||||
|
||||
Manual usage
|
||||
^^^^^^^^^^^^
|
||||
|
||||
You can apply ``ruff`` manually to one or more files with the following
|
||||
command:
|
||||
|
||||
::
|
||||
|
||||
ruff -l 120 <path/to/file(s)>
|
||||
|
||||
- ``-l 120`` means that the allowed number of characters per line is 120.
|
||||
This number was agreed upon by the developers.
|
||||
- The path can point to several files, either one after the other or using
|
||||
wildcards like in a typical Unix shell.
|
||||
|
||||
|
||||
Pre-commit hook
|
||||
~~~~~~~~~~~~~~~
|
||||
|
||||
For ease of use, we provide hooks for Git with the `pre-commit <https://pre-commit.com/>`__
|
||||
Python framework that will run ``ruff`` automatically on all your commits with the
|
||||
correct version of ``ruff``.
|
||||
To set up:
|
||||
|
||||
::
|
||||
|
||||
pip install pre-commit
|
||||
pre-commit install
|
||||
|
||||
|
||||
You can also run the hook manually with ``pre-commit run``.
|
||||
|
||||
.. note::
|
||||
|
||||
Previously, we supplied a hook in the folder ``misc/hooks``. If you copied the
|
||||
script manually, these hooks should still work, but symlinks will be broken.
|
||||
If you are using the new system, run ``rm .git/hooks/*`` to remove the old hooks
|
||||
that are no longer needed.
|
||||
|
||||
|
||||
Editor integration
|
||||
^^^^^^^^^^^^^^^^^^
|
||||
|
||||
Many IDEs or code editors have beautifier plugins that can be configured to run
|
||||
ruff automatically, for example, each time you save a file. For details, you can
|
||||
check `Ruff Editor Integrations <https://docs.astral.sh/ruff/editors/>`__.
|
||||
|
||||
Comment style guide
|
||||
-------------------
|
||||
|
||||
This comment style guide applies to all programming languages used within
|
||||
Godot's codebase.
|
||||
|
||||
- Begin comments with a space character to distinguish them from disabled code.
|
||||
- Use sentence case for comments. Begin comments with an uppercase character and
|
||||
always end them with a period.
|
||||
- Reference variable/function names and values using backticks.
|
||||
- Wrap comments to ~100 characters.
|
||||
- You can use ``TODO:``, ``FIXME:``, ``NOTE:``, ``WARNING:``, or ``HACK:`` as admonitions
|
||||
when needed.
|
||||
|
||||
**Example:**
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
// Compute the first 10,000 decimals of Pi.
|
||||
// FIXME: Don't crash when computing the 1,337th decimal due to `increment`
|
||||
// being negative.
|
||||
|
||||
Don't repeat what the code says in a comment. Explain the *why* rather than *how*.
|
||||
|
||||
**Bad:**
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
// Draw loading screen.
|
||||
draw_load_screen();
|
||||
|
||||
You can use Javadoc-style comments above function or macro definitions. It's
|
||||
recommended to use Javadoc-style comments *only* for methods which are not
|
||||
exposed to scripting. This is because exposed methods should be documented in
|
||||
the :ref:`class reference XML <doc_updating_the_class_reference>`
|
||||
instead.
|
||||
|
||||
**Example:**
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
/**
|
||||
* Returns the number of nodes in the universe.
|
||||
* This can potentially be a very large number, hence the 64-bit return type.
|
||||
*/
|
||||
uint64_t Universe::get_node_count() {
|
||||
// ...
|
||||
}
|
||||
|
||||
For member variables, don't use Javadoc-style comments, but use single-line comments instead:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
class Universe {
|
||||
// The cached number of nodes in the universe.
|
||||
// This value may not always be up-to-date with the current number of nodes
|
||||
// in the universe.
|
||||
uint64_t node_count_cached = 0;
|
||||
};
|
||||
144
engine/compatibility_breakages.rst
Normal file
144
engine/compatibility_breakages.rst
Normal file
@@ -0,0 +1,144 @@
|
||||
.. _doc_handling_compatibility_breakages:
|
||||
|
||||
Handling compatibility breakages
|
||||
================================
|
||||
|
||||
.. TODO: Elaborate on types of compatibility and procedure.
|
||||
|
||||
So you've added a new parameter to a method, changed the return type,
|
||||
changed the type of a parameter, or changed its default value,
|
||||
and now the automated testing is complaining about compatibility breakages?
|
||||
|
||||
Breaking compatibility should be avoided, but when necessary there are systems in place
|
||||
to handle this in a way that makes the transition as smooth as possible.
|
||||
|
||||
A practical example
|
||||
-------------------
|
||||
|
||||
.. TODO: Add example that showcases more details like original default arguments etc.
|
||||
|
||||
These changes are taken from `pull request #88047 <https://github.com/godotengine/godot/pull/88047>`_, which added
|
||||
new pathing options to ``AStarGrid2D`` and other AStar classes.
|
||||
Among other changes, these methods were modified in ``core/math/a_star_grid_2d.h``:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
Vector<Vector2> get_point_path(const Vector2i &p_from, const Vector2i &p_to);
|
||||
TypedArray<Vector2i> get_id_path(const Vector2i &p_from, const Vector2i &p_to);
|
||||
|
||||
To:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
Vector<Vector2> get_point_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false);
|
||||
TypedArray<Vector2i> get_id_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false);
|
||||
|
||||
This meant adding new compatibility method bindings to the file, which should be in the ``protected`` section of
|
||||
the code, usually placed next to ``_bind_methods()``:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
#ifndef DISABLE_DEPRECATED
|
||||
TypedArray<Vector2i> _get_id_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to);
|
||||
Vector<Vector2> _get_point_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to);
|
||||
static void _bind_compatibility_methods();
|
||||
#endif
|
||||
|
||||
They should start with an ``_`` to indicate that they are internal, and end with ``_bind_compat_`` followed by the PR number
|
||||
that introduced the change (``88047`` in this example). These compatibility methods need to be implemented in a dedicated file,
|
||||
like ``core/math/a_star_grid_2d.compat.inc`` in this case:
|
||||
|
||||
.. code-block:: cpp
|
||||
:caption: core/math/a_star_grid_2d.compat.inc
|
||||
|
||||
/**************************************************************************/
|
||||
/* a_star_grid_2d.compat.inc */
|
||||
/**************************************************************************/
|
||||
/* This file is part of: */
|
||||
/* GODOT ENGINE */
|
||||
/* https://godotengine.org */
|
||||
/**************************************************************************/
|
||||
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
|
||||
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
|
||||
/* */
|
||||
/* Permission is hereby granted, free of charge, to any person obtaining */
|
||||
/* a copy of this software and associated documentation files (the */
|
||||
/* "Software"), to deal in the Software without restriction, including */
|
||||
/* without limitation the rights to use, copy, modify, merge, publish, */
|
||||
/* distribute, sublicense, and/or sell copies of the Software, and to */
|
||||
/* permit persons to whom the Software is furnished to do so, subject to */
|
||||
/* the following conditions: */
|
||||
/* */
|
||||
/* The above copyright notice and this permission notice shall be */
|
||||
/* included in all copies or substantial portions of the Software. */
|
||||
/* */
|
||||
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
|
||||
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
|
||||
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
|
||||
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
|
||||
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
|
||||
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
|
||||
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
|
||||
/**************************************************************************/
|
||||
|
||||
#ifndef DISABLE_DEPRECATED
|
||||
|
||||
#include "core/variant/typed_array.h"
|
||||
|
||||
TypedArray<Vector2i> AStarGrid2D::_get_id_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) {
|
||||
return get_id_path(p_from_id, p_to_id, false);
|
||||
}
|
||||
|
||||
Vector<Vector2> AStarGrid2D::_get_point_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) {
|
||||
return get_point_path(p_from_id, p_to_id, false);
|
||||
}
|
||||
|
||||
void AStarGrid2D::_bind_compatibility_methods() {
|
||||
ClassDB::bind_compatibility_method(D_METHOD("get_id_path", "from_id", "to_id"), &AStarGrid2D::_get_id_path_bind_compat_88047);
|
||||
ClassDB::bind_compatibility_method(D_METHOD("get_point_path", "from_id", "to_id"), &AStarGrid2D::_get_point_path_bind_compat_88047);
|
||||
}
|
||||
|
||||
#endif // DISABLE_DEPRECATED
|
||||
|
||||
Unless the change in compatibility is complex, the compatibility method should call the modified method directly,
|
||||
instead of duplicating that method. Make sure to match the default arguments for that method (in the example above this would be ``false``).
|
||||
|
||||
This file should always be placed next to the original file, and have ``.compat.inc`` at the end instead of ``.cpp`` or ``.h``.
|
||||
Next, this should be included in the ``.cpp`` file we're adding compatibility methods to, so ``core/math/a_star_grid_2d.cpp``:
|
||||
|
||||
.. code-block:: cpp
|
||||
:caption: core/math/a_star_grid_2d.cpp
|
||||
|
||||
#include "a_star_grid_2d.h"
|
||||
#include "a_star_grid_2d.compat.inc"
|
||||
|
||||
#include "core/variant/typed_array.h"
|
||||
|
||||
And finally, the changes reported by the API validation step should be added to the relevant validation file. Because this was
|
||||
done during the development of 4.3, this would be ``misc/extension_api_validation/4.2-stable.expected`` (including changes not shown in
|
||||
this example):
|
||||
|
||||
.. code-block:: text
|
||||
:caption: misc/extension_api_validation/4.2-stable.expected
|
||||
|
||||
GH-88047
|
||||
--------
|
||||
Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
|
||||
Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
|
||||
Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
|
||||
Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
|
||||
Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
|
||||
Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
|
||||
|
||||
Added optional "allow_partial_path" argument to get_id_path and get_point_path methods in AStar classes.
|
||||
Compatibility methods registered.
|
||||
|
||||
The instructions for how to add to that file are at the top of the file itself.
|
||||
|
||||
If you get a "Hash changed" error for a method, it means that the compatibility binding is missing or incorrect.
|
||||
Such lines shouldn't be added to the ``.expected`` file, but fixed by binding the proper compatibility method.
|
||||
|
||||
And that's it! You might run into a bit more complicated cases, like rearranging arguments,
|
||||
changing return types, etc., but this covers the basic on how to use this system.
|
||||
|
||||
For more information, see `pull request #76446 <https://github.com/godotengine/godot/pull/76446>`_.
|
||||
130
engine/cpp_usage_guidelines.rst
Normal file
130
engine/cpp_usage_guidelines.rst
Normal file
@@ -0,0 +1,130 @@
|
||||
.. _doc_cpp_usage_guidelines:
|
||||
|
||||
C++ usage guidelines
|
||||
====================
|
||||
|
||||
Rationale
|
||||
---------
|
||||
|
||||
Since Godot 4.0, the C++ standard used throughout the codebase is a subset of
|
||||
**C++17**. While modern C++ brings a lot of opportunities to write faster, more
|
||||
readable code, we chose to restrict our usage of C++ to a subset for a few
|
||||
reasons:
|
||||
|
||||
- It makes it easier to review code in online editors. This is because engine
|
||||
contributors don't always have access to a full-featured IDE while reviewing
|
||||
code.
|
||||
- It makes the code easier to grasp for beginner contributors (who may not be
|
||||
professional C++ programmers). Godot's codebase is known to be easy to learn
|
||||
from, and we'd like to keep it that way.
|
||||
|
||||
To get your pull request merged, it needs to follow the C++ usage guidelines
|
||||
outlined here. Of course, you can use features not allowed here in your own C++
|
||||
modules or GDExtensions.
|
||||
|
||||
.. note::
|
||||
|
||||
Prior to Godot 4.0, the C++ standard used throughout the codebase was C++03,
|
||||
with a handful of C++14 extensions. If you are contributing a pull request
|
||||
to the `3.x` branch rather than `master`, your code can't use C++17 features.
|
||||
Instead, your code must be able to be built with a C++14 compiler.
|
||||
|
||||
The guidelines below don't apply to third-party dependencies, although we
|
||||
generally favor small libraries instead of larger solutions. See also
|
||||
:ref:`doc_best_practices_for_engine_contributors`.
|
||||
|
||||
.. seealso::
|
||||
|
||||
See :ref:`doc_code_style_guidelines` for formatting guidelines.
|
||||
|
||||
Disallowed features
|
||||
-------------------
|
||||
|
||||
**Any feature not listed below is allowed.** Using features like ``constexpr``
|
||||
variables and ``nullptr`` is encouraged when possible. Still, try to keep your
|
||||
use of modern C++ features conservative. Their use needs to serve a real
|
||||
purpose, such as improving code readability or performance.
|
||||
|
||||
.. _doc_cpp_godot_types:
|
||||
|
||||
Standard Template Library
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
We don't allow using the `STL <https://en.wikipedia.org/wiki/Standard_Template_Library>`__
|
||||
as Godot provides its own data types (among other things).
|
||||
See :ref:`doc_faq_why_not_stl` for more information.
|
||||
|
||||
This means that pull requests should **not** use ``std::string``,
|
||||
``std::vector`` and the like. Instead, use Godot's datatypes as described in
|
||||
the :ref:`doc_core_types` documentation.
|
||||
|
||||
A 📜 icon denotes the type is part of :ref:`Variant <doc_variant_class>`. This
|
||||
means it can be used as a parameter or return value of a method exposed to the
|
||||
scripting API.
|
||||
|
||||
``auto`` keyword
|
||||
~~~~~~~~~~~~~~~~
|
||||
|
||||
Please don't use the ``auto`` keyword for type inference. While it can avoid
|
||||
repetition, it can also lead to confusing code:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
// Not so confusing...
|
||||
auto button = memnew(Button);
|
||||
|
||||
// ...but what about this?
|
||||
auto result = EditorNode::get_singleton()->get_complex_result();
|
||||
|
||||
Keep in mind hover documentation often isn't readily available for pull request
|
||||
reviewers. Most of the time, reviewers will use GitHub's online viewer to review
|
||||
pull requests.
|
||||
|
||||
The ``auto`` keyword can be used in some special cases, like C++ lambda or Objective-C block
|
||||
definitions and C++ templates. Please ask before using templates with ``auto`` in a pull request.
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
// Full type definitions.
|
||||
void (*mult64to128)(uint64_t, uint64_t, uint64_t &, uint64_t &) = [](uint64_t u, uint64_t v, uint64_t &h, uint64_t &l) { ... }
|
||||
void (^JOYSTICK_LEFT)(GCControllerDirectionPad *__strong, float, float) = ^(GCControllerDirectionPad *dpad, float xValue, float yValue) { ... }
|
||||
|
||||
// Less clutter with auto.
|
||||
auto mult64to128 = [](uint64_t u, uint64_t v, uint64_t &h, uint64_t &l) { ... }
|
||||
auto JOYSTICK_LEFT = ^(GCControllerDirectionPad *dpad, float xValue, float yValue) { ... }
|
||||
|
||||
// Compare function for different types.
|
||||
template <typename T1, typename T2>
|
||||
constexpr auto MIN(const T1 m_a, const T2 m_b) {
|
||||
return m_a < m_b ? m_a : m_b;
|
||||
}
|
||||
|
||||
We chose to forbid ``auto`` in all other cases. Thank you for your understanding.
|
||||
|
||||
Lambdas
|
||||
~~~~~~~
|
||||
|
||||
Lambdas should be used conservatively when they make code effectively faster or
|
||||
simpler, and do not impede readability. Please ask before using lambdas in a
|
||||
pull request.
|
||||
|
||||
``#ifdef``-based include guards
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Starting with 4.5, all files now use the ``#pragma once`` directive, as they
|
||||
improve readability and declutter macros. Use of ``#ifdef``-based include
|
||||
guards are now actively discouraged.
|
||||
|
||||
``try``-``catch`` blocks
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
C++ style exception handling using ``try`` and ``catch`` blocks is forbidden.
|
||||
This restriction is in place for several reasons, including performance, binary
|
||||
size and code complexity.
|
||||
Use :ref:`doc_common_engine_methods_and_macros_error_macros` instead.
|
||||
|
||||
|
||||
.. seealso::
|
||||
|
||||
See :ref:`doc_code_style_guidelines_header_includes` for guidelines on sorting
|
||||
includes in C++ and Objective-C files.
|
||||
579
engine/creating_pull_requests.rst
Normal file
579
engine/creating_pull_requests.rst
Normal file
@@ -0,0 +1,579 @@
|
||||
.. _doc_pr_workflow:
|
||||
|
||||
Creating pull requests
|
||||
======================
|
||||
|
||||
.. highlight:: shell
|
||||
|
||||
The so-called "PR workflow" used by Godot is common to many projects using
|
||||
Git, and should be familiar to veteran free software contributors. The idea
|
||||
is that only a small number (if any) commit directly to the *master* branch.
|
||||
Instead, contributors *fork* the project (i.e. create a copy of it, which
|
||||
they can modify as they wish), and then use the GitHub interface to request
|
||||
a *pull* from one of their fork's branches to one branch of the original
|
||||
(often named *upstream*) repository.
|
||||
|
||||
The resulting *pull request* (PR) can then be reviewed by other contributors,
|
||||
which might approve it, reject it, or most often request that modifications
|
||||
be done. Once approved, the PR can then be merged by one of the core
|
||||
developers, and its commit(s) will become part of the target branch (usually
|
||||
the *master* branch).
|
||||
|
||||
We will go together through an example to show the typical workflow and
|
||||
associated Git commands. But first, let's have a quick look at the
|
||||
organization of Godot's Git repository.
|
||||
|
||||
Git source repository
|
||||
---------------------
|
||||
|
||||
The `repository on GitHub <https://github.com/godotengine/godot>`_ is a
|
||||
`Git <https://git-scm.com>`_ code repository together with an embedded
|
||||
issue tracker and PR system.
|
||||
|
||||
.. note:: If you are contributing to the documentation, its repository can
|
||||
be found `here <https://github.com/godotengine/godot-docs>`_.
|
||||
|
||||
The Git version control system is the tool used to keep track of successive
|
||||
edits to the source code - to contribute efficiently to Godot, learning the
|
||||
basics of the Git command line is *highly* recommended. There exist some
|
||||
graphical interfaces for Git, but they usually encourage users to take bad
|
||||
habits regarding the Git and PR workflow, and we therefore recommend not to
|
||||
use them. In particular, we advise not to use GitHub's online editor for code
|
||||
contributions (although it's tolerated for small fixes or documentation changes)
|
||||
as it enforces one commit per file and per modification,
|
||||
which quickly leads to PRs with an unreadable Git history (especially after peer review).
|
||||
|
||||
.. seealso:: The first sections of Git's "Book" are a good introduction to
|
||||
the tool's philosophy and the various commands you need to
|
||||
master in your daily workflow. You can read them online on the
|
||||
`Git SCM <https://git-scm.com/book/en/v2>`_ website.
|
||||
You can also try out `GitHub's interactive guide <https://try.github.io/>`__.
|
||||
|
||||
The branches on the Git repository are organized as follows:
|
||||
|
||||
- The ``master`` branch is where the development of the next major version
|
||||
occurs. As a development branch, it can be unstable
|
||||
and is not meant for use in production. This is where PRs should be done
|
||||
in priority.
|
||||
- The stable branches are named after their version, e.g. ``3.1`` and ``2.1``.
|
||||
They are used to backport bugfixes and enhancements from the ``master``
|
||||
branch to the currently maintained stable release (e.g. 3.1.2 or 2.1.6).
|
||||
As a rule of thumb, the last stable branch is maintained until the next
|
||||
minor version (e.g. the ``3.0`` branch was maintained until the release of
|
||||
Godot 3.1).
|
||||
If you want to make PRs against a maintained stable branch, please check
|
||||
first if your changes are also relevant for the ``master`` branch, and if so
|
||||
make the PR for the ``master`` branch in priority. Release managers can then
|
||||
cherry-pick the fix to a stable branch if relevant.
|
||||
- There might occasionally be feature branches, usually meant to be merged into
|
||||
the ``master`` branch at some time.
|
||||
|
||||
Forking and cloning
|
||||
-------------------
|
||||
|
||||
The first step is to *fork* the `godotengine/godot <https://github.com/godotengine/godot>`_
|
||||
repository on GitHub. To do so, you will need to have a GitHub account and to
|
||||
be logged in. In the top right corner of the repository's GitHub page, you
|
||||
should see the "Fork" button as shown below:
|
||||
|
||||
.. image:: img/github_fork_button.png
|
||||
|
||||
Click it, and after a while you should be redirected to your own fork of the
|
||||
Godot repo, with your GitHub username as namespace:
|
||||
|
||||
.. image:: img/github_fork_url.png
|
||||
|
||||
You can then *clone* your fork, i.e. create a local copy of the online
|
||||
repository (in Git speak, the *origin remote*). If you haven't already,
|
||||
download Git from `its website <https://git-scm.com>`_ if you're using Windows or
|
||||
macOS, or install it through your package manager if you're using Linux.
|
||||
|
||||
.. note:: If you are on Windows, open Git Bash to type commands. macOS and Linux users
|
||||
can use their respective terminals.
|
||||
|
||||
To clone your fork from GitHub, use the following command:
|
||||
|
||||
::
|
||||
|
||||
git clone https://github.com/USERNAME/godot
|
||||
|
||||
|
||||
After a little while, you should have a ``godot`` directory in your current
|
||||
working directory. Move into it using the ``cd`` command:
|
||||
|
||||
::
|
||||
|
||||
cd godot
|
||||
|
||||
We will start by setting up a reference to the original repository that we forked:
|
||||
|
||||
::
|
||||
|
||||
git remote add upstream https://github.com/godotengine/godot
|
||||
git fetch upstream
|
||||
|
||||
This will create a reference named ``upstream`` pointing to the original
|
||||
``godotengine/godot`` repository. This will be useful when you want to pull new
|
||||
commits from its ``master`` branch to update your fork. You have another
|
||||
remote reference named ``origin``, which points to your fork (``USERNAME/godot``).
|
||||
|
||||
You only need to do the above steps once, as long as you keep that local
|
||||
``godot`` folder (which you can move around if you want, the relevant
|
||||
metadata is hidden in its ``.git`` subfolder).
|
||||
|
||||
.. note:: *Branch it, pull it, code it, stage it, commit, push it, rebase
|
||||
it... technologic.*
|
||||
|
||||
This bad take on Daft Punk's *Technologic* shows the general
|
||||
conception Git beginners have of its workflow: lots of strange
|
||||
commands to learn by copy and paste, hoping they will work as
|
||||
expected. And that's actually not a bad way to learn, as long as
|
||||
you're curious and don't hesitate to question your search engine
|
||||
when lost, so we will give you the basic commands to know when
|
||||
working in Git.
|
||||
|
||||
In the following, we will assume as an example that you want to implement a feature in
|
||||
Godot's Project Manager, which is coded in the ``editor/project_manager.cpp``
|
||||
file.
|
||||
|
||||
Branching
|
||||
---------
|
||||
|
||||
By default, the ``git clone`` should have put you on the ``master`` branch of
|
||||
your fork (``origin``). To start your own feature development, we will create
|
||||
a feature branch:
|
||||
|
||||
::
|
||||
|
||||
# Create the branch based on the current branch (master)
|
||||
git branch better-project-manager
|
||||
|
||||
# Change the current branch to the new one
|
||||
git checkout better-project-manager
|
||||
|
||||
This command is equivalent:
|
||||
|
||||
::
|
||||
|
||||
# Change the current branch to a new named one, based on the current branch
|
||||
git checkout -b better-project-manager
|
||||
|
||||
If you want to go back to the ``master`` branch, you'd use:
|
||||
|
||||
::
|
||||
|
||||
git checkout master
|
||||
|
||||
You can see which branch you are currently on with the ``git branch``
|
||||
command:
|
||||
|
||||
::
|
||||
|
||||
git branch
|
||||
2.1
|
||||
* better-project-manager
|
||||
master
|
||||
|
||||
Be sure to always go back to the ``master`` branch before creating a new branch,
|
||||
as your current branch will be used as the base for the new one. Alternatively,
|
||||
you can specify a custom base branch after the new branch's name:
|
||||
|
||||
::
|
||||
|
||||
git checkout -b my-new-feature master
|
||||
|
||||
Updating your branch
|
||||
--------------------
|
||||
|
||||
This would not be needed the first time (just after you forked the upstream
|
||||
repository). However, the next time you want to work on something, you will
|
||||
notice that your fork's ``master`` is several commits behind the upstream
|
||||
``master`` branch: pull requests from other contributors would have been merged
|
||||
in the meantime.
|
||||
|
||||
To ensure there won't be conflicts between the feature you develop and the
|
||||
current upstream ``master`` branch, you will have to update your branch by
|
||||
*pulling* the upstream branch.
|
||||
|
||||
::
|
||||
|
||||
git pull --rebase upstream master
|
||||
|
||||
The ``--rebase`` argument will ensure that any local changes that you committed
|
||||
will be re-applied *on top* of the pulled branch, which is usually what we want
|
||||
in our PR workflow. This way, when you open a pull request, your own commits will
|
||||
be the only difference with the upstream ``master`` branch.
|
||||
|
||||
While rebasing, conflicts may arise if your commits modified code that has been
|
||||
changed in the upstream branch in the meantime. If that happens, Git will stop at
|
||||
the conflicting commit and will ask you to resolve the conflicts. You can do so
|
||||
with any text editor, then stage the changes (more on that later), and proceed with
|
||||
``git rebase --continue``. Repeat the operation if later commits have conflicts too,
|
||||
until the rebase operation completes.
|
||||
|
||||
If you're unsure about what is going on during a rebase and you panic (no worry,
|
||||
we all do the first few times), you can abort the rebase with ``git rebase --abort``.
|
||||
You will then be back to the original state of your branch before calling
|
||||
``git pull --rebase``.
|
||||
|
||||
.. note:: If you omit the ``--rebase`` argument, you will instead create a merge
|
||||
commit which tells Git what to make of the two distinct branches. If any
|
||||
conflicts arise, they would be resolved all at once via this merge commit.
|
||||
|
||||
While this is a valid workflow and the default behavior of ``git pull``,
|
||||
merge commits within PRs are frowned upon in our PR workflow. We only use
|
||||
them when merging PRs into the upstream branch.
|
||||
|
||||
The philosophy is that a PR should represent the final stage of the changes
|
||||
made to the codebase, and we are not interested in mistakes and fixes that
|
||||
would have been done in intermediate stages before merging.
|
||||
Git gives us great tools to "rewrite the history" and make it as if we got
|
||||
things right the first time, and we're happy to use it to ensure that
|
||||
changes are easy to review and understand long after they have been merged.
|
||||
|
||||
If you have already created a merge commit without using ``rebase``, or
|
||||
have made any other changes that have resulted in undesired history, the best option
|
||||
is to use an *interactive rebase* on the upstream branch. See the :ref:`dedicated
|
||||
section <doc_pr_workflow_rebase>` for instructions.
|
||||
|
||||
.. tip:: If at any time you want to *reset* a local branch to a given commit or branch,
|
||||
you can do so with ``git reset --hard <commit ID>`` or
|
||||
``git reset --hard <remote>/<branch>`` (e.g. ``git reset --hard upstream/master``).
|
||||
|
||||
Be warned that this will remove any changes that you might have committed in
|
||||
this branch. If you ever lose commits by mistake, use the ``git reflog`` command
|
||||
to find the commit ID of the previous state that you would like to restore, and
|
||||
use it as argument of ``git reset --hard`` to go back to that state.
|
||||
|
||||
Making changes
|
||||
--------------
|
||||
|
||||
You would then do your changes to our example's
|
||||
``editor/project_manager.cpp`` file with your usual development environment
|
||||
(text editor, IDE, etc.).
|
||||
|
||||
By default, those changes are *unstaged*. The staging area is a layer between
|
||||
your working directory (where you make your modifications) and the local Git
|
||||
repository (the commits and all the metadata in the ``.git`` folder). To
|
||||
bring changes from the working directory to the Git repository, you need to
|
||||
*stage* them with the ``git add`` command, and then to commit them with the
|
||||
``git commit`` command.
|
||||
|
||||
There are various commands you should know to review your current work,
|
||||
before staging it, while it is staged, and after it has been committed.
|
||||
|
||||
- ``git diff`` will show you the current unstaged changes, i.e. the
|
||||
differences between your working directory and the staging area.
|
||||
- ``git checkout -- <files>`` will undo the unstaged changes to the given
|
||||
files.
|
||||
- ``git add <files>`` will *stage* the changes on the listed files.
|
||||
- ``git diff --staged`` will show the current staged changes, i.e. the
|
||||
differences between the staging area and the last commit.
|
||||
- ``git reset HEAD <files>`` will *unstage* changes to the listed files.
|
||||
- ``git status`` will show you what are the currently staged and unstaged
|
||||
modifications.
|
||||
- ``git commit`` will commit the staged files. It will open a text editor
|
||||
(you can define the one you want to use with the ``GIT_EDITOR`` environment
|
||||
variable or the ``core.editor`` setting in your Git configuration) to let you
|
||||
write a commit log. You can use ``git commit -m "Cool commit log"`` to
|
||||
write the log directly.
|
||||
- ``git commit --amend`` lets you amend the last commit with your currently
|
||||
staged changes (added with ``git add``). This is the best option if you
|
||||
want to fix a mistake in the last commit (bug, typo, style issue, etc.).
|
||||
- ``git log`` will show you the last commits of your current branch. If you
|
||||
did local commits, they should be shown at the top.
|
||||
- ``git show`` will show you the changes of the last commit. You can also
|
||||
specify a commit hash to see the changes for that commit.
|
||||
|
||||
That's a lot to memorize! Don't worry, just check this cheat sheet when you
|
||||
need to make changes, and learn by doing.
|
||||
|
||||
Here's how the shell history could look like on our example:
|
||||
|
||||
::
|
||||
|
||||
# It's nice to know where you're starting from
|
||||
git log
|
||||
|
||||
# Do changes to the Project Manager with the nano text editor
|
||||
nano editor/project_manager.cpp
|
||||
|
||||
# Find an unrelated bug in Control and fix it
|
||||
nano scene/gui/control.cpp
|
||||
|
||||
# Review changes
|
||||
git status
|
||||
git diff
|
||||
|
||||
# We'll do two commits for our unrelated changes,
|
||||
# starting by the Control changes necessary for the PM enhancements
|
||||
git add scene/gui/control.cpp
|
||||
git commit -m "Fix handling of margins in Control"
|
||||
|
||||
# Check we did good
|
||||
git log
|
||||
git show
|
||||
git status
|
||||
|
||||
# Make our second commit
|
||||
git add editor/project_manager.cpp
|
||||
git commit -m "Add a pretty banner to the Project Manager"
|
||||
git log
|
||||
|
||||
With this, we should have two new commits in our ``better-project-manager``
|
||||
branch which were not in the ``master`` branch. They are still only local
|
||||
though, the remote fork does not know about them, nor does the upstream repo.
|
||||
|
||||
Pushing changes to a remote
|
||||
---------------------------
|
||||
|
||||
That's where ``git push`` will come into play. In Git, a commit is always
|
||||
done in the local repository (unlike Subversion where a commit will modify
|
||||
the remote repository directly). You need to *push* the new commits to a
|
||||
remote branch to share them with the world. The syntax for this is:
|
||||
|
||||
::
|
||||
|
||||
git push <remote> <local branch>[:<remote branch>]
|
||||
|
||||
The part about the remote branch can be omitted if you want it to have the
|
||||
same name as the local branch, which is our case in this example, so we will
|
||||
do:
|
||||
|
||||
::
|
||||
|
||||
git push origin better-project-manager
|
||||
|
||||
Git will ask you for your username and password. For your password, enter your
|
||||
GitHub Personal Access Token (PAT). If you do not have a GitHub Personal Access
|
||||
Token, or do not have one with the correct permissions for your newly forked
|
||||
repository, you will need to create one. Follow this link to create your Personal
|
||||
Access Token: `Creating a personal access token
|
||||
<https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token>`_.
|
||||
|
||||
After you have successfully verified your account using your PAT, the changes
|
||||
will be sent to your remote repository. If you check the fork's page on GitHub,
|
||||
you should see a new branch with your added commits.
|
||||
|
||||
Issuing a pull request
|
||||
----------------------
|
||||
|
||||
When you load your fork's branch on GitHub, you should see a line saying
|
||||
*"This branch is 2 commits ahead of godotengine:master."* (and potentially some
|
||||
commits behind, if your ``master`` branch was out of sync with the upstream
|
||||
``master`` branch).
|
||||
|
||||
.. image:: img/github_fork_make_pr.png
|
||||
|
||||
On that line, there is a "Pull request" link. Clicking it will open a form
|
||||
that will let you issue a pull request on the ``godotengine/godot`` upstream
|
||||
repository. It should show you your two commits, and state "Able to merge".
|
||||
If not (e.g. it has way more commits, or says there are merge conflicts),
|
||||
don't create the PR yet, something went wrong. Go to our
|
||||
`Godot Contributors Chat <https://chat.godotengine.org/>`_ and ask for support :)
|
||||
|
||||
Use an explicit title for the PR and put the necessary details in the comment
|
||||
area. You can drag and drop screenshots, GIFs or zipped projects if relevant,
|
||||
to showcase what your work implements. Click "Create a pull request", and
|
||||
tadaa!
|
||||
|
||||
Modifying a pull request
|
||||
------------------------
|
||||
|
||||
While it is reviewed by other contributors, you will often need to make
|
||||
changes to your yet-unmerged PR, either because contributors requested them,
|
||||
or because you found issues yourself while testing.
|
||||
|
||||
The good news is that you can modify a pull request simply by acting on the
|
||||
branch you made the pull request from. You can e.g. make a new commit on that
|
||||
branch, push it to your fork, and the PR will be updated automatically:
|
||||
|
||||
::
|
||||
|
||||
# Check out your branch again if you had changed in the meantime
|
||||
git checkout better-project-manager
|
||||
|
||||
# Fix a mistake
|
||||
nano editor/project_manager.cpp
|
||||
git add editor/project_manager.cpp
|
||||
git commit -m "Fix a typo in the banner's title"
|
||||
git push origin better-project-manager
|
||||
|
||||
However, be aware that in our PR workflow, we favor commits that bring the
|
||||
codebase from one functional state to another functional state, without having
|
||||
intermediate commits fixing up bugs in your own code or style issues. Most of
|
||||
the time, we will prefer a single commit in a given PR (unless there's a good
|
||||
reason to keep the changes separate). Instead of authoring a new commit,
|
||||
consider using ``git commit --amend`` to amend the previous commit with your
|
||||
fixes. The above example would then become:
|
||||
|
||||
::
|
||||
|
||||
# Check out your branch again if you had changed in the meantime
|
||||
git checkout better-project-manager
|
||||
|
||||
# Fix a mistake
|
||||
nano editor/project_manager.cpp
|
||||
git add editor/project_manager.cpp
|
||||
# --amend will change the previous commit, so you will have the opportunity
|
||||
# to edit its commit message if relevant.
|
||||
git commit --amend
|
||||
# As we modified the last commit, it no longer matches the one from your
|
||||
# remote branch, so we need to force push to overwrite that branch.
|
||||
git push --force origin better-project-manager
|
||||
|
||||
.. Kept for compatibility with the previous title, linked in many PRs.
|
||||
|
||||
.. _mastering-the-pr-workflow-the-rebase:
|
||||
|
||||
.. _doc_pr_workflow_rebase:
|
||||
|
||||
The interactive rebase
|
||||
----------------------
|
||||
|
||||
If you didn't follow the above steps closely to *amend* changes into a commit
|
||||
instead of creating fixup commits, or if you authored your changes without being
|
||||
aware of our workflow and Git usage tips, reviewers might request you to
|
||||
*rebase* your branch to *squash* some or all of the commits into one.
|
||||
|
||||
Indeed, if some commits have been made following reviews to fix bugs, typos, etc.
|
||||
in the original commit, they are not relevant to a future changelog reader who
|
||||
would want to know what happened in the Godot codebase, or when and how a given
|
||||
file was last modified.
|
||||
|
||||
To squash those extraneous commits into the main one, we will have to *rewrite
|
||||
history*. Right, we have that power. You may read that it's a bad practice, and
|
||||
it's true when it comes to branches of the upstream repo. But in your fork, you
|
||||
can do whatever you want, and everything is allowed to get neat PRs :)
|
||||
|
||||
We will use the *interactive rebase* ``git rebase -i`` to do this. This
|
||||
command takes a commit ID or a branch name as argument, and will let you modify
|
||||
all commits between that commit/branch and the last one in your working branch,
|
||||
the so-called ``HEAD``.
|
||||
|
||||
While you can give any commit ID to ``git rebase -i`` and review everything in
|
||||
between, the most common and convenient workflow involves rebasing on the
|
||||
upstream ``master`` branch, which you can do with:
|
||||
|
||||
::
|
||||
|
||||
git rebase -i upstream/master
|
||||
|
||||
.. note:: Referencing branches in Git is a bit tricky due to the distinction
|
||||
between remote and local branches. Here, ``upstream/master`` (with a
|
||||
`/`) is a local branch which has been pulled from the ``upstream``
|
||||
remote's ``master`` branch.
|
||||
|
||||
Interactive rebases can only be done on local branches, so the `/`
|
||||
is important here. As the upstream remote changes frequently, your
|
||||
local ``upstream/master`` branch may become outdated, so you can
|
||||
update it with ``git fetch upstream master``. Contrarily to
|
||||
``git pull --rebase upstream master`` which would update your
|
||||
currently checked out branch, ``fetch`` will only update the
|
||||
``upstream/master`` reference (which is distinct from your local
|
||||
``master`` branch... yes it's confusing, but you'll become familiar
|
||||
with this little by little).
|
||||
|
||||
This will open a text editor (``vi`` by default, see
|
||||
`Git docs <https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration#_core_editor>`_
|
||||
to configure your favorite one) with something which may look like this:
|
||||
|
||||
.. code-block:: text
|
||||
|
||||
pick 1b4aad7 Add a pretty banner to the Project Manager
|
||||
pick e07077e Fix a typo in the banner's title
|
||||
|
||||
The editor will also show instructions regarding how you can act on those
|
||||
commits. In particular, it should tell you that "pick" means to use that
|
||||
commit (do nothing), and that "squash" and "fixup" can be used to *meld* the
|
||||
commit in its parent commit. The difference between "squash" and "fixup" is
|
||||
that "fixup" will discard the commit log from the squashed commit. In our
|
||||
example, we are not interested in keeping the log of the "Fix a typo" commit,
|
||||
so we use:
|
||||
|
||||
.. code-block:: text
|
||||
|
||||
pick 1b4aad7 Add a pretty banner to the Project Manager
|
||||
fixup e07077e Fix a typo in the banner's title
|
||||
|
||||
Upon saving and quitting the editor, the rebase will occur. The second commit
|
||||
will be melded into the first one, and ``git log`` and ``git show`` should
|
||||
now confirm that you have only one commit with the changes from both previous
|
||||
commits.
|
||||
|
||||
But! You rewrote the history, and now your local and remote branches have
|
||||
diverged. Indeed, commit 1b4aad7 in the above example will have changed, and
|
||||
therefore got a new commit hash. If you try to push to your remote branch, it
|
||||
will raise an error:
|
||||
|
||||
::
|
||||
|
||||
git push origin better-project-manager
|
||||
To https://github.com/akien-mga/godot
|
||||
! [rejected] better-project-manager -> better-project-manager (non-fast-forward)
|
||||
error: failed to push some refs to 'https://akien-mga@github.com/akien-mga/godot'
|
||||
hint: Updates were rejected because the tip of your current branch is behind
|
||||
hint: its remote counterpart.
|
||||
|
||||
This is reasonable behavior, Git will not let you push changes that would
|
||||
override remote content. But that's actually what we want to do here, so we
|
||||
will have to *force* it:
|
||||
|
||||
::
|
||||
|
||||
git push --force origin better-project-manager
|
||||
|
||||
And tadaa! Git will happily *replace* your remote branch with what you had
|
||||
locally (so make sure that's what you wanted, using ``git log``). This will
|
||||
also update the PR accordingly.
|
||||
|
||||
Rebasing onto another branch
|
||||
----------------------------
|
||||
|
||||
If you have accidentally opened your PR on the wrong branch, or need to target another branch
|
||||
for some reason, you might need to filter out a lot of commits that differ between the old branch
|
||||
(for example ``4.2``) and the new branch (for example ``master``). This can make rebasing difficult
|
||||
and tedious. Fortunately ``git`` has a command just for this situation, ``git rebase --onto``.
|
||||
|
||||
If your PR was created from the ``4.2`` branch and you want to update it to instead start at ``master``
|
||||
the following steps *should* fix this in one step:
|
||||
|
||||
.. code-block:: text
|
||||
|
||||
git rebase -i --onto master 4.2
|
||||
|
||||
This will take all the commits on your branch *after* the ``4.2`` branch, and then splice them on top of ``master``,
|
||||
ignoring any commits from the ``4.2`` branch not on the ``master`` branch. You may still need to do some fixing, but
|
||||
this command should save you a lot of tedious work removing commits.
|
||||
|
||||
Just like above for the interactive rebase you need to force push your branch to handle the different changes:
|
||||
|
||||
::
|
||||
|
||||
git push --force origin better-project-manager
|
||||
|
||||
Deleting a Git branch
|
||||
---------------------
|
||||
|
||||
After your pull request gets merged, there's one last thing you should do: delete your
|
||||
Git branch for the PR. There won't be issues if you don't delete your branch, but it's
|
||||
good practice to do so. You'll need to do this twice, once for the local branch and another
|
||||
for the remote branch on GitHub.
|
||||
|
||||
To delete our better Project Manager branch locally, use this command:
|
||||
|
||||
::
|
||||
|
||||
git branch -d better-project-manager
|
||||
|
||||
Alternatively, if the branch hadn't been merged yet and we wanted to delete it anyway, instead
|
||||
of ``-d`` you would use ``-D``.
|
||||
|
||||
Next, to delete the remote branch on GitHub use this command:
|
||||
|
||||
::
|
||||
|
||||
git push origin -d better-project-manager
|
||||
|
||||
You can also delete the remote branch from the GitHub PR itself, a button should appear once
|
||||
it has been merged or closed.
|
||||
60
engine/first_steps.rst
Normal file
60
engine/first_steps.rst
Normal file
@@ -0,0 +1,60 @@
|
||||
First Steps
|
||||
===========
|
||||
|
||||
The possibility to study, use, modify and redistribute modifications of the
|
||||
engine's source code are the fundamental rights that
|
||||
Godot's `MIT <https://tldrlegal.com/license/mit-license>`_ license grants you,
|
||||
making it `free and open source software <https://en.wikipedia.org/wiki/Free_and_open-source_software>`_.
|
||||
|
||||
As such, everyone is entitled to modify
|
||||
`Godot's source code <https://github.com/godotengine/godot>`_, and send those
|
||||
modifications back to the upstream project in the form of a patch (a text file
|
||||
describing the changes in a ready-to-apply manner) or - in the modern workflow
|
||||
that we use - via a so-called "pull request" (PR), i.e. a proposal to directly
|
||||
merge one or more Git commits (patches) into the main development branch.
|
||||
|
||||
Contributing code changes upstream has two big advantages:
|
||||
|
||||
- Your own code will be reviewed and improved by other developers, and will be
|
||||
further maintained directly in the upstream project, so you won't have to
|
||||
reapply your own changes every time you move to a newer version. On the
|
||||
other hand it comes with a responsibility, as your changes have to be
|
||||
generic enough to be beneficial to all users, and not just your project; so
|
||||
in some cases it might still be relevant to keep your changes only for your
|
||||
own project, if they are too specific.
|
||||
|
||||
- The whole community will benefit from your work, and other contributors will
|
||||
behave the same way, contributing code that will be beneficial to you. At
|
||||
the time of this writing, over 2,000 developers have contributed code
|
||||
changes to the engine!
|
||||
|
||||
To ensure good collaboration and overall quality, the Godot developers
|
||||
enforce some rules for code contributions, for example regarding the style to
|
||||
use in the C++ code (indentation, brackets, etc.) or the Git and PR workflow.
|
||||
|
||||
A good place to start is by searching for issues tagged as
|
||||
`good first issue <https://github.com/godotengine/godot/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22>`_
|
||||
on GitHub.
|
||||
|
||||
.. seealso:: Technical details about the PR workflow are outlined in a
|
||||
specific section, :ref:`doc_pr_workflow`.
|
||||
|
||||
Details about the code style guidelines and the ``clang-format``
|
||||
tool used to enforce them are outlined in
|
||||
:ref:`doc_code_style_guidelines`.
|
||||
|
||||
All pull requests must go through a review process before being accepted.
|
||||
Depending on the scope of the changes, it may take some time for a maintainer
|
||||
responsible for the modified part of the engine to provide their review.
|
||||
We value all of our contributors and ask them to be patient in the meantime,
|
||||
as it is expected that in an open source project like Godot, there is going to be
|
||||
way more contributions than people validating them.
|
||||
|
||||
To make sure that your time and efforts aren't wasted, it is recommended to vet the idea
|
||||
first before implementing it and putting it for a review as a PR. To that end, Godot
|
||||
has a `proposal system <https://github.com/godotengine/godot-proposals>`_. Its
|
||||
usage is encouraged to plan changes and discuss them with the community. Implementation
|
||||
details can also be discussed with other contributors on the `Godot Contributors Chat <https://chat.godotengine.org/>`_.
|
||||
|
||||
.. note:: Proposals are only required when working on an enhancement or a new feature.
|
||||
Bug reports are sufficient for fixing issues.
|
||||
455
engine/unit_tests.rst
Normal file
455
engine/unit_tests.rst
Normal file
@@ -0,0 +1,455 @@
|
||||
.. _doc_unit_testing:
|
||||
|
||||
Unit testing
|
||||
============
|
||||
|
||||
Godot Engine allows to write unit tests directly in C++. The engine integrates
|
||||
the `doctest <https://github.com/doctest/doctest>`_ unit testing framework which
|
||||
gives ability to write test suites and test cases next to production code, but
|
||||
since the tests in Godot go through a different ``main`` entry point, the tests
|
||||
reside in a dedicated ``tests/`` directory instead, which is located at the root
|
||||
of the engine source code.
|
||||
|
||||
Platform and target support
|
||||
---------------------------
|
||||
|
||||
C++ unit tests can be run on Linux, macOS, and Windows operating systems.
|
||||
|
||||
Tests can only be run with editor ``tools`` enabled, which means that export
|
||||
templates cannot be tested currently.
|
||||
|
||||
Running tests
|
||||
-------------
|
||||
|
||||
Before tests can be actually run, the engine must be compiled with the ``tests``
|
||||
build option enabled (and any other build option you typically use), as the
|
||||
tests are not compiled as part of the engine by default:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
scons tests=yes
|
||||
|
||||
Once the build is done, run the tests with a ``--test`` command-line option:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test
|
||||
|
||||
The test run can be configured with the various doctest-specific command-line
|
||||
options. To retrieve the full list of supported options, run the ``--test``
|
||||
command with the ``--help`` option:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --help
|
||||
|
||||
Any other options and arguments after the ``--test`` command are treated as
|
||||
arguments for doctest.
|
||||
|
||||
.. note::
|
||||
|
||||
Tests are compiled automatically if you use the ``dev_mode=yes`` SCons option.
|
||||
``dev_mode=yes`` is recommended if you plan on contributing to the engine
|
||||
development as it will automatically treat compilation warnings as errors.
|
||||
The continuous integration system will fail if any compilation warnings are
|
||||
detected, so you should strive to fix all warnings before opening a pull
|
||||
request.
|
||||
|
||||
Filtering tests
|
||||
~~~~~~~~~~~~~~~
|
||||
|
||||
By default, all tests are run if you don't supply any extra arguments after the
|
||||
``--test`` command. But if you're writing new tests or would like to see the
|
||||
successful assertions output coming from those tests for debugging purposes, you
|
||||
can run the tests of interest with the various filtering options provided by
|
||||
doctest.
|
||||
|
||||
The wildcard syntax ``*`` is supported for matching any number of characters in
|
||||
test suites, test cases, and source file names:
|
||||
|
||||
+--------------------+---------------+------------------------+
|
||||
| **Filter options** | **Shorthand** | **Examples** |
|
||||
+--------------------+---------------+------------------------+
|
||||
| ``--test-suite`` | ``-ts`` | ``-ts="*[GDScript]*"`` |
|
||||
+--------------------+---------------+------------------------+
|
||||
| ``--test-case`` | ``-tc`` | ``-tc="*[String]*"`` |
|
||||
+--------------------+---------------+------------------------+
|
||||
| ``--source-file`` | ``-sf`` | ``-sf="*test_color*"`` |
|
||||
+--------------------+---------------+------------------------+
|
||||
|
||||
For instance, to run only the ``String`` unit tests, run:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --test-case="*[String]*"
|
||||
|
||||
Successful assertions output can be enabled with the ``--success`` (``-s``)
|
||||
option, and can be combined with any combination of filtering options above,
|
||||
for instance:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --source-file="*test_color*" --success
|
||||
|
||||
Specific tests can be skipped with corresponding ``-exclude`` options. As of
|
||||
now, some tests include random stress tests which take a while to execute. In
|
||||
order to skip those kind of tests, run the following command:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --test-case-exclude="*[Stress]*"
|
||||
|
||||
Writing tests
|
||||
-------------
|
||||
|
||||
Test suites represent C++ header files which must be included as part of the
|
||||
main test entry point in ``tests/test_main.cpp``. Most test suites are located
|
||||
directly under ``tests/`` directory.
|
||||
|
||||
All header files are prefixed with ``test_``, and this is a naming convention
|
||||
which the Godot build system relies on to detect tests throughout the engine.
|
||||
|
||||
Here's a minimal working test suite with a single test case written:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "tests/test_macros.h"
|
||||
|
||||
namespace TestString {
|
||||
|
||||
TEST_CASE("[String] Hello World!") {
|
||||
String hello = "Hello World!";
|
||||
CHECK(hello == "Hello World!");
|
||||
}
|
||||
|
||||
} // namespace TestString
|
||||
|
||||
.. note::
|
||||
You can quickly generate new tests using the ``create_test.py`` script found in the ``tests/`` directory.
|
||||
This script automatically creates a new test file with the required boilerplate code in the appropriate location.
|
||||
It's also able to automatically include the new header in ``tests/test_main.cpp`` using invasive mode (``-i`` flag).
|
||||
To view usage instructions, run the script with the ``-h`` flag.
|
||||
|
||||
The ``tests/test_macros.h`` header encapsulates everything which is needed for
|
||||
writing C++ unit tests in Godot. It includes doctest assertion and logging
|
||||
macros such as ``CHECK`` as seen above, and of course the definitions for
|
||||
writing test cases themselves.
|
||||
|
||||
.. seealso::
|
||||
|
||||
`tests/test_macros.h <https://github.com/godotengine/godot/blob/master/tests/test_macros.h>`_
|
||||
source code for currently implemented macros and aliases for them.
|
||||
|
||||
Test cases are created using ``TEST_CASE`` function-like macro. Each test case
|
||||
must have a brief description written in parentheses, optionally including
|
||||
custom tags which allow to filter the tests at runtime, such as ``[String]``,
|
||||
``[Stress]`` etc.
|
||||
|
||||
Test cases are written in a dedicated namespace. This is not required, but
|
||||
allows to prevent naming collisions for when other static helper functions are
|
||||
written to accommodate the repeating testing procedures such as populating
|
||||
common test data for each test, or writing parameterized tests.
|
||||
|
||||
Godot supports writing tests per C++ module. For instructions on how to write
|
||||
module tests, refer to :ref:`doc_custom_module_unit_tests`.
|
||||
|
||||
Subcases
|
||||
~~~~~~~~
|
||||
|
||||
In situations where you have a common setup for several test cases with only slight variations, subcases can be very helpful. Here's an example:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
TEST_CASE("[SceneTree][Node] Testing node operations with a very simple scene tree") {
|
||||
// ... common setup (e.g. creating a scene tree with a few nodes)
|
||||
SUBCASE("Move node to specific index") {
|
||||
// ... setup and checks for moving a node
|
||||
}
|
||||
SUBCASE("Remove node at specific index") {
|
||||
// ... setup and checks for removing a node
|
||||
}
|
||||
}
|
||||
|
||||
Each ``SUBCASE`` causes the ``TEST_CASE`` to be executed from the beginning.
|
||||
Subcases can be nested to an arbitrary depth, but it is advised to limit nesting to no more than one level deep.
|
||||
|
||||
Assertions
|
||||
~~~~~~~~~~
|
||||
|
||||
A list of all commonly used assertions used throughout the Godot tests, sorted
|
||||
by severity.
|
||||
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| **Assertion** | **Description** |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``REQUIRE`` | Test if condition holds true. Fails the entire test immediately if the condition does not hold true. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``REQUIRE_FALSE`` | Test if condition does not hold true. Fails the entire test immediately if the condition holds true. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``CHECK`` | Test if condition holds true. Marks the test run as failing, but allow to run other assertions. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``CHECK_FALSE`` | Test if condition does not hold true. Marks the test run as failing, but allow to run other assertions. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``WARN`` | Test if condition holds true. Does not fail the test under any circumstance, but logs a warning if something does not hold true. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``WARN_FALSE`` | Test if condition does not hold true. Does not fail the test under any circumstance, but logs a warning if something holds true. |
|
||||
+-------------------+----------------------------------------------------------------------------------------------------------------------------------+
|
||||
|
||||
All of the above assertions have corresponding ``*_MESSAGE`` macros, which allow
|
||||
to print optional message with rationale of what should happen.
|
||||
|
||||
Prefer to use ``CHECK`` for self-explanatory assertions and ``CHECK_MESSAGE``
|
||||
for more complex ones if you think that it deserves a better explanation.
|
||||
|
||||
.. seealso::
|
||||
|
||||
`doctest: Assertion macros <https://github.com/doctest/doctest/blob/master/doc/markdown/assertions.md>`_.
|
||||
|
||||
Logging
|
||||
~~~~~~~
|
||||
|
||||
The test output is handled by doctest itself, and does not rely on Godot
|
||||
printing or logging functionality at all, so it's recommended to use dedicated
|
||||
macros which allow to log test output in a format written by doctest.
|
||||
|
||||
+----------------+-----------------------------------------------------------------------------------------------------------+
|
||||
| **Macro** | **Description** |
|
||||
+----------------+-----------------------------------------------------------------------------------------------------------+
|
||||
| ``MESSAGE`` | Prints a message. |
|
||||
+----------------+-----------------------------------------------------------------------------------------------------------+
|
||||
| ``FAIL_CHECK`` | Marks the test as failing, but continue the execution. Can be wrapped in conditionals for complex checks. |
|
||||
+----------------+-----------------------------------------------------------------------------------------------------------+
|
||||
| ``FAIL`` | Fails the test immediately. Can be wrapped in conditionals for complex checks. |
|
||||
+----------------+-----------------------------------------------------------------------------------------------------------+
|
||||
|
||||
Different reporters can be chosen at runtime. For instance, here's how the
|
||||
output can be redirected to an XML file:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --source-file="*test_validate*" --success --reporters=xml --out=doctest.txt
|
||||
|
||||
.. seealso::
|
||||
|
||||
`doctest: Logging macros <https://github.com/doctest/doctest/blob/master/doc/markdown/logging.md>`_.
|
||||
|
||||
Testing failure paths
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Sometimes, it's not always feasible to test for an *expected* result. With the
|
||||
Godot development philosophy of that the engine should not crash and should
|
||||
gracefully recover whenever a non-fatal error occurs, it's important to check
|
||||
that those failure paths are indeed safe to execute without crashing the engine.
|
||||
|
||||
*Unexpected* behavior can be tested in the same way as anything else. The only
|
||||
problem this creates is that the error printing shall unnecessarily pollute the
|
||||
test output with errors coming from the engine itself (even if the end result is
|
||||
successful).
|
||||
|
||||
To alleviate this problem, use ``ERR_PRINT_OFF`` and ``ERR_PRINT_ON`` macros
|
||||
directly within test cases to temporarily disable the error output coming from
|
||||
the engine, for instance:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
TEST_CASE("[Color] Constructor methods") {
|
||||
ERR_PRINT_OFF;
|
||||
Color html_invalid = Color::html("invalid");
|
||||
ERR_PRINT_ON; // Don't forget to re-enable!
|
||||
|
||||
CHECK_MESSAGE(html_invalid.is_equal_approx(Color()),
|
||||
"Invalid HTML notation should result in a Color with the default values.");
|
||||
}
|
||||
|
||||
Special tags in test case names
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
These tags can be added to the test case name to modify or extend the test environment:
|
||||
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| **Tag** | **Description** |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``[SceneTree]`` | Required for test cases that rely on a scene tree with MessageQueue to be available. It also enables a mock rendering server and :ref:`ThemeDB<class_ThemeDB>`. |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``[Editor]`` | Like ``[SceneTree]``, but with additional editor-related infrastructure available, such as :ref:`EditorSettings<class_EditorSettings>`. |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``[Audio]`` | Initializes the :ref:`AudioServer<class_AudioServer>` using a mock audio driver. |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``[Navigation2D]`` | Creates the default 2D navigation server and makes it available for testing. |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
| ``[Navigation3D]`` | Creates the default 3D navigation server and makes it available for testing. |
|
||||
+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
||||
|
||||
You can use them together to combine multiple test environment extensions.
|
||||
|
||||
Testing signals
|
||||
~~~~~~~~~~~~~~~
|
||||
|
||||
The following macros can be use to test signals:
|
||||
|
||||
.. list-table::
|
||||
:header-rows: 1
|
||||
:widths: auto
|
||||
|
||||
* - Macro
|
||||
- Description
|
||||
* - ``SIGNAL_WATCH(object, "signal_name")``
|
||||
- Starts watching the specified signal on the given object.
|
||||
* - ``SIGNAL_UNWATCH(object, "signal_name")``
|
||||
- Stops watching the specified signal on the given object.
|
||||
* - ``SIGNAL_CHECK("signal_name", Vector<Vector<Variant>>)``
|
||||
- Checks the arguments of all fired signals. The outer vector contains each fired signal, while the inner vector contains the list of arguments for that signal. The order of signals is significant.
|
||||
* - ``SIGNAL_CHECK_FALSE("signal_name")``
|
||||
- Checks if the specified signal was not fired.
|
||||
* - ``SIGNAL_DISCARD("signal_name")``
|
||||
- Discards all records of the specified signal.
|
||||
|
||||
Below is an example demonstrating the use of these macros:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
//...
|
||||
SUBCASE("[Timer] Timer process timeout signal must be emitted") {
|
||||
SIGNAL_WATCH(test_timer, SNAME("timeout"));
|
||||
test_timer->start(0.1);
|
||||
|
||||
SceneTree::get_singleton()->process(0.2);
|
||||
|
||||
Array signal_args;
|
||||
signal_args.push_back(Array());
|
||||
|
||||
SIGNAL_CHECK(SNAME("timeout"), signal_args);
|
||||
|
||||
SIGNAL_UNWATCH(test_timer, SNAME("timeout"));
|
||||
}
|
||||
//...
|
||||
|
||||
Test tools
|
||||
----------
|
||||
|
||||
Test tools are advanced methods which allow you to run arbitrary procedures to
|
||||
facilitate the process of manual testing and debugging the engine internals.
|
||||
|
||||
These tools can be run by supplying the name of a tool after the ``--test``
|
||||
command-line option. For instance, the GDScript module implements and registers
|
||||
several tools to help the debugging of the tokenizer, parser, and compiler:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test gdscript-tokenizer test.gd
|
||||
./bin/<godot_binary> --test gdscript-parser test.gd
|
||||
./bin/<godot_binary> --test gdscript-compiler test.gd
|
||||
|
||||
If any such tool is detected, then the rest of the unit tests are skipped.
|
||||
|
||||
Test tools can be registered anywhere throughout the engine as the registering
|
||||
mechanism closely resembles of what doctest provides while registering test
|
||||
cases using dynamic initialization technique, but usually these can be
|
||||
registered at corresponding ``register_types.cpp`` sources (per module or core).
|
||||
|
||||
Here's an example of how GDScript registers test tools in
|
||||
``modules/gdscript/register_types.cpp``:
|
||||
|
||||
.. code-block:: cpp
|
||||
|
||||
#ifdef TESTS_ENABLED
|
||||
void test_tokenizer() {
|
||||
TestGDScript::test(TestGDScript::TestType::TEST_TOKENIZER);
|
||||
}
|
||||
|
||||
void test_parser() {
|
||||
TestGDScript::test(TestGDScript::TestType::TEST_PARSER);
|
||||
}
|
||||
|
||||
void test_compiler() {
|
||||
TestGDScript::test(TestGDScript::TestType::TEST_COMPILER);
|
||||
}
|
||||
|
||||
REGISTER_TEST_COMMAND("gdscript-tokenizer", &test_tokenizer);
|
||||
REGISTER_TEST_COMMAND("gdscript-parser", &test_parser);
|
||||
REGISTER_TEST_COMMAND("gdscript-compiler", &test_compiler);
|
||||
#endif
|
||||
|
||||
The custom command-line parsing can be performed by a test tool itself with the
|
||||
help of OS :ref:`get_cmdline_args<class_OS_method_get_cmdline_args>` method.
|
||||
|
||||
Integration tests for GDScript
|
||||
------------------------------
|
||||
|
||||
Godot uses doctest to prevent regressions in GDScript during development. There
|
||||
are several types of test scripts which can be written:
|
||||
|
||||
- tests for expected errors;
|
||||
- tests for warnings;
|
||||
- tests for features.
|
||||
|
||||
Therefore, the process of writing integration tests for GDScript is the following:
|
||||
|
||||
1. Pick a type of a test script you'd like to write, and create a new GDScript
|
||||
file under the ``modules/gdscript/tests/scripts`` directory within
|
||||
corresponding sub-directory.
|
||||
|
||||
2. Write GDScript code. The test script must have a function called ``test()``
|
||||
which takes no arguments. Such function will be called by the test runner.
|
||||
The test should not have any dependency unless it's part of the test too.
|
||||
Global classes (using ``class_name``) are registered before the runner
|
||||
starts, so those should work if needed.
|
||||
|
||||
Here's an example test script:
|
||||
|
||||
::
|
||||
|
||||
func test():
|
||||
if true # Missing colon here.
|
||||
print("true")
|
||||
|
||||
3. Change directory to the Godot source repository root.
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
cd godot
|
||||
|
||||
4. Generate ``*.out`` files to update the expected results from the output:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
bin/<godot_binary> --gdscript-generate-tests modules/gdscript/tests/scripts
|
||||
|
||||
You may add the ``--print-filenames`` option to see filenames as their test
|
||||
outputs are generated. If you are working on a new feature that is causing
|
||||
hard crashes, you can use this option to quickly find which test file causes
|
||||
the crash and debug from there.
|
||||
|
||||
5. Run GDScript tests with:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
./bin/<godot_binary> --test --test-suite="*GDScript*"
|
||||
|
||||
This also accepts the ``--print-filenames`` option (see above).
|
||||
|
||||
If no errors are printed and everything goes well, you're done!
|
||||
|
||||
.. warning::
|
||||
|
||||
Make sure the output does have the expected values before submitting a pull
|
||||
request. If ``--gdscript-generate-tests`` produces ``*.out`` files which are
|
||||
unrelated to newly added tests, you should revert those files back and
|
||||
only commit ``*.out`` files for new tests.
|
||||
|
||||
.. note::
|
||||
|
||||
The GDScript test runner is meant for testing the GDScript implementation,
|
||||
not for testing user scripts nor testing the engine using scripts. We
|
||||
recommend writing new tests for already resolved
|
||||
`issues related to GDScript at GitHub <https://github.com/godotengine/godot/issues?q=is%3Aissue+label%3Atopic%3Agdscript+is%3Aclosed>`_,
|
||||
or writing tests for currently working features.
|
||||
|
||||
.. note::
|
||||
|
||||
If your test case requires that there is no ``test()``
|
||||
function present inside the script file,
|
||||
you can disable the runtime section of the test by naming the script file so that it matches the pattern ``*.notest.gd``.
|
||||
For example, "test_empty_file.notest.gd".
|
||||
43
index.rst
43
index.rst
@@ -9,23 +9,33 @@ TODO
|
||||
.. toctree::
|
||||
:hidden:
|
||||
:maxdepth: 1
|
||||
:caption: Contributing Source Code
|
||||
:name: sec-contributing-source-code
|
||||
:caption: Contributing to the engine
|
||||
:name: sec-engine
|
||||
|
||||
contributing_source_code/first_steps
|
||||
contributing_source_code/creating_pull_requests
|
||||
contributing_source_code/reviewing_pull_requests
|
||||
contributing_source_code/code_style
|
||||
contributing_source_code/documentation
|
||||
contributing_source_code/unit_tests
|
||||
engine/first_steps
|
||||
engine/creating_pull_requests
|
||||
engine/cpp_usage_guidelines
|
||||
engine/code_style
|
||||
engine/compatibility_breakages
|
||||
engine/unit_tests
|
||||
|
||||
.. toctree::
|
||||
:hidden:
|
||||
:maxdepth: 1
|
||||
:caption: Contributing Translations
|
||||
:name: sec-translations
|
||||
:caption: Contributing documentation
|
||||
:name: sec-engine
|
||||
|
||||
translation/first_steps
|
||||
documentation/first_steps
|
||||
documentation/translation
|
||||
|
||||
.. toctree::
|
||||
:hidden:
|
||||
:maxdepth: 1
|
||||
:caption: Sharing ideas and proposing changes
|
||||
:name: sec-proposals
|
||||
|
||||
proposals/about
|
||||
proposals/guidelines
|
||||
|
||||
.. toctree::
|
||||
:hidden:
|
||||
@@ -34,19 +44,22 @@ TODO
|
||||
:name: sec-triage
|
||||
|
||||
triage/about
|
||||
triage/bisecting
|
||||
|
||||
.. toctree::
|
||||
:hidden:
|
||||
:maxdepth: 1
|
||||
:caption: Godot Implementation Proposals
|
||||
:name: sec-proposals
|
||||
:caption: Pull Request Review
|
||||
:name: sec-pull-request-review
|
||||
|
||||
proposals/about
|
||||
pull_request_review/process
|
||||
pull_request_review/guidelines
|
||||
pull_request_review/testing
|
||||
|
||||
.. toctree::
|
||||
:hidden:
|
||||
:maxdepth: 1
|
||||
:caption: Maintainers Documentation
|
||||
:caption: Maintainers documentation
|
||||
:name: sec-maintainers
|
||||
|
||||
maintainers/about
|
||||
|
||||
@@ -1,2 +1,2 @@
|
||||
About Godot Maintainers
|
||||
About Godot maintainers
|
||||
=======================
|
||||
|
||||
@@ -1,2 +0,0 @@
|
||||
About Godot Implementation Proposals
|
||||
====================================
|
||||
2
proposals/first_steps.rst
Normal file
2
proposals/first_steps.rst
Normal file
@@ -0,0 +1,2 @@
|
||||
First steps
|
||||
===========
|
||||
242
proposals/guidelines.rst
Normal file
242
proposals/guidelines.rst
Normal file
@@ -0,0 +1,242 @@
|
||||
.. _doc_best_practices_for_engine_contributors:
|
||||
|
||||
Proposal guidelines
|
||||
===================
|
||||
|
||||
Introduction
|
||||
------------
|
||||
|
||||
Godot has a large amount of users who have the ability to contribute because the
|
||||
project itself is aimed mainly at users who can code. That being said, not all
|
||||
of them have the same level of experience working in large projects or in
|
||||
software engineering, which can lead to common misunderstandings and bad
|
||||
practices during the process of contributing code to the project.
|
||||
|
||||
Language
|
||||
--------
|
||||
|
||||
The scope of this document is to be a list of best practices for contributors to
|
||||
follow, as well as to create a language they can use to refer to common
|
||||
situations that arise in the process of submitting their contributions.
|
||||
|
||||
While a generalized list of software development best practices might be useful,
|
||||
we'll focus on the situations that are most common in our project.
|
||||
|
||||
Contributions are most of the time categorized as bug fixes, enhancements or new
|
||||
features. To abstract this idea, we will call them *Solutions*, because they
|
||||
always seek to solve something that can be described as a *Problem*.
|
||||
|
||||
Best Practices
|
||||
--------------
|
||||
|
||||
#1: The problem always comes first
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Many contributors are extremely creative and just enjoy the process of designing
|
||||
abstract data structures, creating nice user interfaces, or simply love
|
||||
programming. Whatever the case may be, they come up with cool ideas, which may
|
||||
or may not solve real problems.
|
||||
|
||||
.. image:: img/best_practices1.png
|
||||
|
||||
These are usually called *solutions in search of a problem*. In an ideal world,
|
||||
they would not be harmful but, in reality, code takes time to write, takes up
|
||||
space and requires maintenance once it exists. Avoiding the addition of anything
|
||||
unnecessary is always considered a good practice in software development.
|
||||
|
||||
#2: To solve the problem, it has to exist in the first place
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
This is a variation of the previous practice. Adding anything unnecessary is not
|
||||
a good idea, but what constitutes what is necessary and what isn't?
|
||||
|
||||
.. image:: img/best_practices2.png
|
||||
|
||||
The answer to this question is that the problem needs to *exist* before it can
|
||||
be actually solved. It must not be speculation or a belief. The user must be
|
||||
using the software as intended to create something they *need*. In this process,
|
||||
the user may stumble upon a problem that requires a solution to proceed, or in
|
||||
order to achieve greater productivity. In this case, *a solution is needed*.
|
||||
|
||||
Believing that problems may arise in the future and that the software needs to
|
||||
be ready to solve them by the time they appear is called *"Future proofing"* and
|
||||
it's characterized by lines of thought such as:
|
||||
|
||||
- I think it would be useful for users to...
|
||||
- I think users will eventually need to...
|
||||
|
||||
This is generally considered a bad habit because trying to solve problems that
|
||||
*don't actually exist* in the present will often lead to code that will be
|
||||
written but never used, or that is considerably more complex to use and maintain
|
||||
than it needs to be.
|
||||
|
||||
#3: The problem has to be complex or frequent
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Software is designed to solve problems, but we can't expect it to solve *every
|
||||
problem that exists under the sun*. As a game engine, Godot will help you make
|
||||
games better and faster, but it won't make an *entire game* for you. A line must
|
||||
be drawn somewhere.
|
||||
|
||||
.. image:: img/best_practices3.png
|
||||
|
||||
Whether a problem is worth solving is determined by the effort that is required
|
||||
to work around it. The required effort depends on:
|
||||
|
||||
- The complexity of the problem
|
||||
- The frequency the problem
|
||||
|
||||
If the problem is *too complex* for most users to solve, then the software
|
||||
should offer a ready-made solution for it. Likewise, if the problem is easy for
|
||||
the user to work around, offering such a solution is unnecessary.
|
||||
|
||||
The exception, however, is when the user encounters a problem *frequently
|
||||
enough* that having to do the simple solution every time becomes an annoyance.
|
||||
In this case, the software should offer a solution to simplify the use case.
|
||||
|
||||
It's usually easy to tell if a problem is complex or frequent, but it can be
|
||||
difficult. This is why discussing with other developers (next point) is always
|
||||
advised.
|
||||
|
||||
#4: The solution must be discussed with others
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Often, users will be immersed in their own projects when they stumble upon
|
||||
problems. These users will naturally try to solve the problem from their
|
||||
perspective, thinking only about their own use case. As a result, user proposed
|
||||
solutions don't always contemplate all use cases and are often biased towards
|
||||
the user's own requirements.
|
||||
|
||||
.. image:: img/best_practices4.png
|
||||
|
||||
For developers, the perspective is different. They may find the user's problem
|
||||
too unique to justify a solution (instead of a workaround), or they might
|
||||
suggest a partial (usually simpler or lower level) solution that applies to a
|
||||
wider range of known problems and leave the rest of the solution up to the
|
||||
user.
|
||||
|
||||
In any case, before attempting to contribute, it is important to discuss the
|
||||
actual problems with the other developers or contributors, so a better agreement
|
||||
on implementation can be reached.
|
||||
|
||||
The only exception is when an area of code has a clear agreed upon owner, who
|
||||
talks to users directly and has the most knowledge to implement a solution
|
||||
directly.
|
||||
|
||||
Also, Godot's philosophy is to favor ease of use and maintenance over absolute
|
||||
performance. Performance optimizations will be considered, but they may not
|
||||
be accepted if they make something too difficult to use or if they add too much
|
||||
complexity to the codebase.
|
||||
|
||||
#5: To each problem, its own solution
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
For programmers, it is always a most enjoyable challenge to find the most
|
||||
optimal solutions to problems. It is possible to go overboard, though.
|
||||
Sometimes, contributors will try to come up with solutions that solve as many
|
||||
problems as possible.
|
||||
|
||||
The situation will often take a turn for the worse when, in order to make this
|
||||
solution appear even more fantastic and flexible, the pure speculation-based
|
||||
problems (as described in #2) also make their appearance on stage.
|
||||
|
||||
.. image:: img/best_practices5.png
|
||||
|
||||
The main problem is that, in reality, it rarely works this way. Most of the
|
||||
time, writing an individual solution to each problem results in code that
|
||||
is simpler and more maintainable.
|
||||
|
||||
Additionally, solutions that target individual problems are better for the
|
||||
users. Targeted solutions allow users find something that does exactly what they
|
||||
need, without having to learn a more complex system they will only need for simple
|
||||
tasks.
|
||||
|
||||
Big and flexible solutions also have an additional drawback which is that, over
|
||||
time, they are rarely flexible enough for all users. Users end up requesting
|
||||
more and more functionality which ends up making the API and codebase
|
||||
more and more complex.
|
||||
|
||||
#6: Cater to common use cases, leave the door open for the rare ones
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
This is a continuation of the previous point, which further explains why this
|
||||
way of thinking and designing software is preferred.
|
||||
|
||||
As mentioned before (in point #2), it is very difficult for us (as human beings
|
||||
who design software) to actually understand all future user needs. Trying to
|
||||
write very flexible structures that cater to many use cases at once is often a
|
||||
mistake.
|
||||
|
||||
We may come up with something we believe is brilliant, but later find out that
|
||||
users will never even use half of it or that they require features that don't
|
||||
quite fit into our original design, forcing us to either throw it away
|
||||
or make it even more complex.
|
||||
|
||||
The question is then, how do we design software that both allows users to do
|
||||
*what we know they need to do* now and allows them to do *what we don't yet know
|
||||
they'll need to do* in the future?
|
||||
|
||||
.. image:: img/best_practices6.png
|
||||
|
||||
The answer to this question is that, to ensure users still can do what they want
|
||||
to do, we need to give them access to a *low-level API* that they can use to
|
||||
achieve what they want, even if it's more work for them because it means
|
||||
reimplementing some logic that already exists.
|
||||
|
||||
In real-life scenarios, these use cases will be at most rare and uncommon
|
||||
anyway, so it makes sense a custom solution needs to be written. This is why
|
||||
it's important to still provide users the basic building blocks to do it.
|
||||
|
||||
#7: Prefer local solutions
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
When looking for a solution to a problem, be it implementing a new feature or
|
||||
fixing a bug, sometimes the easiest path is to add data or a new function in the
|
||||
core layers of code.
|
||||
|
||||
The main problem here is, adding something to the core layers that will only be
|
||||
used from a single location far away will not only make the code more difficult
|
||||
to follow (split in two), but also make the core API larger, more complex, more
|
||||
difficult to understand in general.
|
||||
|
||||
This is bad, because readability and cleanness of core APIs is always of extreme
|
||||
importance given how much code relies on it, and because it's key for new
|
||||
contributors as a starting point to learning the codebase.
|
||||
|
||||
|
||||
.. image:: img/best_practices7.png
|
||||
|
||||
|
||||
A common reason for wanting to do this is that it's usually less code to simply
|
||||
add a hack in the core layers.
|
||||
|
||||
Doing so is not advised. Generally, the code for a solution should be closer to
|
||||
where the problem originates, even if it involves additional, duplicated, more
|
||||
complex, or less efficient code. More creativity might be needed, but this path
|
||||
is always the advised one.
|
||||
|
||||
#8: Don't use complex canned solutions for simple problems
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Not every problem has a simple solution and, many times, the right choice is to
|
||||
use a third-party library to solve the problem.
|
||||
|
||||
As Godot requires to be shipped in a large amount of platforms, we can't
|
||||
link libraries dynamically. Instead, we bundle them in our source tree.
|
||||
|
||||
.. image:: img/best_practices8.png
|
||||
|
||||
As a result, we are very picky with what goes in, and we tend to prefer smaller
|
||||
libraries (single header ones are our favorite). We will only bundle something
|
||||
larger if there is no other choice.
|
||||
|
||||
.. _doc_best_practices_for_engine_contributors_license_compliance:
|
||||
|
||||
Libraries must use a permissive enough license to be included into Godot.
|
||||
Some examples of acceptable licenses are Apache 2.0, BSD, MIT, ISC, and MPL 2.0.
|
||||
In particular, we cannot accept libraries licensed under the GPL or LGPL since
|
||||
these licenses effectively disallow static linking in proprietary software
|
||||
(which Godot is distributed as in most exported projects). This requirement also
|
||||
applies to the editor, since we may want to run it on iOS in the long term.
|
||||
Since iOS doesn't support dynamic linking, static linking is the only option on
|
||||
that platform.
|
||||
369
pull_request_review/guidelines.rst
Normal file
369
pull_request_review/guidelines.rst
Normal file
@@ -0,0 +1,369 @@
|
||||
.. _doc_pr_review_guidelines:
|
||||
|
||||
Guidelines
|
||||
==========
|
||||
|
||||
Code review and testing
|
||||
-----------------------
|
||||
|
||||
The following is a list of things that contributors and engine maintainers can
|
||||
do to conduct a substantive code review of a pull request.
|
||||
|
||||
.. note::
|
||||
If you want to conduct a code review, but can't do everything on this list,
|
||||
say that in your review comment. For example, it is still very helpful to
|
||||
provide comments on code, even if you can't build the pull request locally to
|
||||
test the pull request (or vice versa). Feel free to review the code, just
|
||||
remember to make a note at the end of your review that you have reviewed the
|
||||
code only and have not tested the changes locally.
|
||||
|
||||
1. Confirm that the problem exists
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
PRs need to solve problems and problems need to be documented. Make sure that
|
||||
the pull request links and closes (or at least addresses) a bug or a proposal.
|
||||
If it doesn't, consider asking the contributor to update the opening message of
|
||||
the PR to explain the problem that the PR aims to solve in more detail.
|
||||
|
||||
.. note::
|
||||
It should be clear _why_ a pull request is needed before it is merged. This
|
||||
assists reviewers in determining whether a PR does what it says it does and it
|
||||
helps contributors in the future understand why the code is the way it is.
|
||||
|
||||
2. Test the PR and look for regressions
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
While strict code review and CI help to ensure that all pull requests work as
|
||||
intended, mistakes happen and sometimes contributors push code that creates a
|
||||
problem in addition to solving a problem. Maintainers will avoid merging code
|
||||
that contains a regression even if it solves the problem as intended.
|
||||
|
||||
When reviewing a pull request, ensure that the PR does what it says it does
|
||||
(i.e. fixes the linked bug or implements the new feature) and nothing outside of
|
||||
the PR target area is broken by the change. You can do this by running the
|
||||
editor and trying out some common functions of the editor (adding objects to a
|
||||
scene, running GDScript, opening and closing menus etc.). Also, while reviewing
|
||||
the code, look for suspicious changes in other parts of the engine. Sometimes
|
||||
during rebasing changes slip through that contributors are not aware of.
|
||||
|
||||
3. Do a code review
|
||||
~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Code reviews are usually done by people who are already experienced in a given
|
||||
area. They may be able to provide ideas to make code faster, more organized, or
|
||||
more idiomatic. But, even if you are not very experienced, you may want to
|
||||
conduct a code review to provide feedback within the scope of what you are
|
||||
comfortable reviewing. Doing so is valuable for the area maintainer (as a second
|
||||
set of eyes on a problem is always helpful) and it is also helpful for you as it
|
||||
will help you get more familiar with that area of code and will expose you to
|
||||
how other people solve problems. In fact, reviewing the code of experienced
|
||||
engine maintainers is a great way to get to know the codebase.
|
||||
|
||||
Here are some things to think about and look out for as you review the code:
|
||||
|
||||
* **Code only touches the areas announced in the PR (and the commit
|
||||
message).**
|
||||
|
||||
It can be tempting to fix random things in the code, as you see them. However,
|
||||
this can quickly make a pull request difficult to review and can make it hard
|
||||
to dig through in the commit history. Small touch-ups next to the related area
|
||||
are alright, but often bugs that you can find along the way are better fixed
|
||||
in their own PRs.
|
||||
|
||||
* **Code properly uses Godot's own APIs and patterns.**
|
||||
|
||||
Consistency is very important, and a solution that already exists in the
|
||||
codebase is preferable to an ad-hoc solution.
|
||||
|
||||
* **Are core areas affected by the change?**
|
||||
|
||||
Sometimes a PR that is supposed to solve a local problem can have a
|
||||
far-reaching effect way outside of its scope. Usually it is best to keep code
|
||||
changes local to where the problem arises. If you think that the solution
|
||||
requires changes outside the scope of the problem, it is usually best to seek
|
||||
the opinion of a team leader who may have another idea for how to solve the
|
||||
problem.
|
||||
|
||||
4. Iterate with the contributor and improve the PR
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Maintainers should provide feedback and suggestions for improvement if they spot
|
||||
things in the code that they would like changed. Preferably, suggestions should
|
||||
come in order of importance: first, address overall code design and the approach
|
||||
to solving the problem, then make sure the code is complying with the engine's
|
||||
best practices, and lastly, do the :ref:`code style review <doc_code_style_review>`.
|
||||
|
||||
.. note::
|
||||
|
||||
**Communicate barriers to merging early in the review process.**
|
||||
|
||||
If the PR has clear blockers or will likely not get merged for whatever other
|
||||
reason, that fact should be communicated as early and clearly as possible. We
|
||||
want to avoid stringing people along because it feels bad to say "sorry, no".
|
||||
|
||||
As you review pull requests, keep the Godot `Code of Conduct
|
||||
<https://godotengine.org/code-of-conduct>`_ in mind. Especially the following:
|
||||
|
||||
* Politeness is expected at all times. Be kind and courteous.
|
||||
|
||||
* Always assume positive intent from others.
|
||||
|
||||
* Feedback is always welcome, but keep your criticism constructive.
|
||||
|
||||
Here are some things to avoid as you iterate on a pull request with a
|
||||
contributor:
|
||||
|
||||
* **Needless double reviews.**
|
||||
|
||||
In other words, review the full PR at once and avoid coming back endless times
|
||||
to point out issues that you could have noted in the first review. Of course,
|
||||
this can't always be avoided, but we should try to catch everything at once.
|
||||
|
||||
* **Being overly nitpicky.**
|
||||
|
||||
Code quality can be flexible depending on the area of the engine you are
|
||||
working in. In general, our standard for code quality is much higher in core
|
||||
areas and in performance-sensitive areas than it is in editor code for
|
||||
example.
|
||||
|
||||
* **Expanding the scope of a pull request.**
|
||||
|
||||
Providing context or related/similar issues or proposals that may be fixed
|
||||
similarly can be helpful, but adding a "may as well fix that thing over there
|
||||
as well while at it" or "could we add to this as well?" isn't always fair to
|
||||
the contributor. Use your judgement when deciding whether additional fixes are
|
||||
within scope, but try to keep the scope as close to the original pull request
|
||||
as possible.
|
||||
|
||||
And ultimately, don't feel pressured to deal with the PR all alone. Feel free to
|
||||
ask for a helping hand on the `Godot Contributors Chat
|
||||
<https://chat.godotengine.org>`_, in the appropriate channel or in #general.
|
||||
Other teams may already be tagged for review, so you can also wait or ask for
|
||||
their assistance.
|
||||
|
||||
5. Approve the pull request
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
After reviewing the code, if you think that the code is ready to be merged into
|
||||
the engine, then go ahead and "approve" it. Make sure to also comment and
|
||||
specify the nature of your review (i.e. say whether you ran the code locally,
|
||||
whether you reviewed for style as well as correctness, etc.). Even if you are
|
||||
not an engine maintainer, approving a pull request signals to others that the
|
||||
code is good and likely solves the problem the PR says it does. Approving a pull
|
||||
request as a non-engine maintainer does not guarantee that the code will be
|
||||
merged, other people will still review it, so don't be shy.
|
||||
|
||||
.. _doc_code_style_review:
|
||||
|
||||
Code style review
|
||||
-----------------
|
||||
|
||||
Generally speaking, we aim to conduct a code review before a style/clarity
|
||||
review as contributors typically want to know if their general approach is
|
||||
acceptable before putting in the effort to make nitpicky changes to style. In
|
||||
other words, maintainers shouldn't ask contributors to change the style of code
|
||||
that may need to be rewritten in subsequent reviews. Similarly, maintainers
|
||||
should avoid asking for contributors to rebase PRs if the PR has not been
|
||||
reviewed.
|
||||
|
||||
That being said, not everyone feels confident enough to provide a review on code
|
||||
correctness, in that case, providing comments on code style and clarity ahead of
|
||||
a more substantive code review is totally appropriate and more than welcome.
|
||||
|
||||
In practice the code style review can be done as part of the substantive code
|
||||
review. The important thing is that both the substantive code and the code style
|
||||
need to be reviewed and considered before a pull request is merged.
|
||||
|
||||
When reviewing code style pay particular attention to ensuring that the pull
|
||||
request follows the :ref:`doc_code_style_guidelines`. While ``clang-format`` and
|
||||
various CI checks can catch a lot of inconsistencies, they are far from perfect
|
||||
and are unable to detect some issues. For example, you should check that:
|
||||
|
||||
* The style of header includes is respected.
|
||||
* Identifiers use ``snake_case`` and follow our naming conventions.
|
||||
* Method parameters start with ``p_*`` or ``r_*`` (if they are used to return
|
||||
a value).
|
||||
* Braces are used appropriately, even for one-liner conditionals.
|
||||
* Code is properly spaced (exactly one empty line between methods, no
|
||||
unnecessary empty lines inside of method bodies).
|
||||
|
||||
.. note::
|
||||
|
||||
This list is not complete and doesn't aim to be complete. Refer to
|
||||
the linked style guide document for a complete set of rules. Keep
|
||||
in mind that ``clang-format`` may not catch things you hope it would,
|
||||
so pay attention and try to build a sense of what exactly it can and
|
||||
cannot detect.
|
||||
|
||||
Merging pull requests
|
||||
---------------------
|
||||
|
||||
In general, pull requests should only be merged by members of the production
|
||||
team or team leaders for pull requests in their area of the engine. For example,
|
||||
the networking team leader could merge a networking pull request that doesn't
|
||||
substantially change non-networking sections of code.
|
||||
|
||||
In practice it is best to wait for a member of the production team to merge the
|
||||
pull request as they keep a close eye on the entire codebase and will likely
|
||||
have a better sense of what other recent/upcoming changes this pull request may
|
||||
conflict with (or any other reason that it may make sense to delay the pull
|
||||
request). Feel free to leave a comment saying that the PR should be ready to
|
||||
merge.
|
||||
|
||||
The following are the steps to take before merging a pull request. The degree to
|
||||
which you adhere to these steps can be flexible for simple/straightforward pull
|
||||
requests, but they should be carefully taken for complex or risky pull requests.
|
||||
|
||||
As a contributor you can help move a pull request forward by doing some of these
|
||||
steps yourself.
|
||||
|
||||
1. Get feedback from the right people/teams
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Production team members should ensure that the right people look at a pull
|
||||
request before it is merged. In some cases this may require multiple people to
|
||||
weigh in. In other cases, only one substantive approval is needed before the
|
||||
code can be merged.
|
||||
|
||||
In general, try not to merge things based on one review alone, especially if it
|
||||
is your own. Get a second opinion from another maintainer, and make sure all the
|
||||
teams that may be impacted have been reasonably represented by the reviewers.
|
||||
For example, if a pull request adds to the documentation, it's often useful to
|
||||
let the area maintainers check it for factual correctness and let documentation
|
||||
maintainers check it for formatting, style, and grammar.
|
||||
|
||||
A good rule of thumb is that at least one subject matter expert should have
|
||||
approved the pull request for correctness, and at least one other maintainer
|
||||
should have approved the pull request for code style. Either of those people
|
||||
could be the person merging the pull request.
|
||||
|
||||
Make sure that the reviews and approvals were left by people competent in that
|
||||
specific engine area. It is possible that even a long-standing member of the
|
||||
Godot organization left a review without having the relevant expertise.
|
||||
|
||||
.. note::
|
||||
|
||||
An easy way to find PRs that may be ready for merging is filtering by
|
||||
approved PRs and sorting by recently updated. For example, in the main Godot
|
||||
repository, you can use `this link
|
||||
<https://github.com/godotengine/godot/pulls?q=is%3Apr+is%3Aopen+review%3Aapproved+sort%3Aupdated-desc>`_.
|
||||
|
||||
2. Get feedback from the community
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
If a pull request is having trouble attracting reviewers, you may need to reach
|
||||
out more broadly to ask for help reviewing. Consider asking:
|
||||
|
||||
* the person who reported the bug if the pull request fixes the bug for them,
|
||||
* contributors who have recently edited that file if they could take a look, or
|
||||
* a more experienced maintainer from another area if they could provide feedback.
|
||||
|
||||
3. Git checklist
|
||||
~~~~~~~~~~~~~~~~
|
||||
|
||||
* **Make sure that the PR comes in one commit.**
|
||||
|
||||
When each commit is self-contained and could be used to build a clean and
|
||||
working version of the engine, it may be okay to merge a pull request with
|
||||
multiple commits, but in general, we require that all pull requests only have
|
||||
one commit. This helps us keep the Git history clean.
|
||||
|
||||
* **Fixes made during the review process must be squashed into
|
||||
the main commit.**
|
||||
|
||||
For multi-commit PRs check that those fixes are amended in the relevant
|
||||
commits, and are not just applied on top of everything.
|
||||
|
||||
* **Make sure that the PR has no merge conflicts.**
|
||||
|
||||
Contributors may need to rebase their changes on top of the relevant branch
|
||||
(e.g. ``master`` or ``3.x``) and manually fix merge conflicts. Even if there
|
||||
are no merge conflicts, contributors may need to rebase especially old PRs as
|
||||
the GitHub conflict checker may not catch all conflicts, or the CI may have
|
||||
changed since it was originally run.
|
||||
|
||||
* **Check for proper commit attribution.**
|
||||
|
||||
If a contributor uses an author signature that is not listed in their GitHub
|
||||
account, GitHub won't link the merged pull request to their account. This
|
||||
keeps them from getting proper credit in the GitHub history and makes them
|
||||
appear like a new contributor on the GitHub UI even after several
|
||||
contributions.
|
||||
|
||||
Ultimately, it's up to the user if they want to fix it, but they can do so by
|
||||
authoring the Git commit with the same email they use for their GitHub
|
||||
account, or by adding the email they used for the Git commit to their GitHub
|
||||
profile.
|
||||
|
||||
* **Check for proper commit messages.**
|
||||
|
||||
While we don't have a very strict ruleset for commit messages, we still
|
||||
require them to be short yet descriptive and use proper English. As a
|
||||
maintainer you've probably written them enough times to know how to make one,
|
||||
but for a general template think about *"Fix <issue> in <part of codebase>"*.
|
||||
For a more detailed recommendation see the `contributing.md
|
||||
<https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind>`_
|
||||
page in the main Godot repository.
|
||||
|
||||
4. GitHub checklist
|
||||
~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
* **Validate the target branch of the PR.**
|
||||
|
||||
Most Godot development happens around in the ``master`` branch. Therefore most
|
||||
pull requests must be made against it. From there pull requests can then be
|
||||
backported to other branches. Be wary of people making PRs on the version they
|
||||
are using (e.g, ``3.3``) and guide them to make a change against a
|
||||
higher-order branch (e.g. ``3.x``). If the change is not applicable for the
|
||||
``master`` branch, the initial PR can be made against the current maintenance
|
||||
branch, such as ``3.x``. It's okay for people to make multiple PRs for each
|
||||
target branch, especially if the changes cannot be easily backported.
|
||||
Cherry-picking is also an option, if possible. Use the appropriate labels if
|
||||
the PR can be cherrypicked (e.g. ``cherrypick:3.x``).
|
||||
|
||||
.. note::
|
||||
|
||||
It is possible to change the target branch of the PR, that has already been
|
||||
submitted, but be aware of the consequences. As it cannot be synchronized
|
||||
with the push, the target branch change will inevitable tag the entire list
|
||||
of maintainers for review. It may also render the CI incapable of running
|
||||
properly. A push should help with that, but if nothing else, recommend
|
||||
opening a new, fresh PR.
|
||||
|
||||
* **Make sure that the appropriate milestone is assigned.**
|
||||
|
||||
This will make it more obvious which version would include the submitted
|
||||
changes, should the pull request be merged now. Note, that the milestone is
|
||||
not a binding contract and does not guarantee that this version is definitely
|
||||
going to include the PR. If the pull request is not merged before the version
|
||||
is released, the milestone will be moved (and the PR itself may require a
|
||||
target branch change).
|
||||
|
||||
Similarly, when merging a PR with a higher milestone than the current version,
|
||||
or a "wildcard" milestone (e.g. "4.x"), ensure to update the milestone to the
|
||||
current version.
|
||||
|
||||
* **Make sure that the opening message of the PR contains the
|
||||
magic words "Closes #..." or "Fixes #...".**
|
||||
|
||||
These link the PR and the referenced issue together and allow GitHub to
|
||||
auto-close the latter when you merge the changes. Note, that this only works
|
||||
for the PRs that target the ``master`` branch. For others you need to pay
|
||||
attention and close the related issues manually. Do it with *"Fixed by #..."*
|
||||
or *"Resolved by #..."* comment to clearly indicate the act for future
|
||||
contributors.
|
||||
|
||||
* **For the issues that get closed by the PR add the closest
|
||||
relevant milestone.**
|
||||
|
||||
In other words, if the PR is targeting the ``master`` branch, but is then also
|
||||
cherrypicked for ``3.x``, the next ``3.x`` release would be the appropriate
|
||||
milestone for the closed issue.
|
||||
|
||||
5. Merge the pull request
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
If it is appropriate for you to be merging a pull request (i.e. you are on the
|
||||
production team or you are the team leader for that area), you are confident
|
||||
that the pull request has been sufficiently reviewed, and once you carry out
|
||||
these steps you can go ahead and merge the pull request.
|
||||
54
pull_request_review/process.rst
Normal file
54
pull_request_review/process.rst
Normal file
@@ -0,0 +1,54 @@
|
||||
Review Process
|
||||
==============
|
||||
|
||||
.. note::
|
||||
|
||||
This page is intended to provide insight into the pull request (PR) review
|
||||
process that we aspire to. As such, it is primarily targeted at engine
|
||||
maintainers who are responsible for reviewing and approving pull requests.
|
||||
That being said, much of the content is useful for prospective contributors
|
||||
wanting to know how to ensure that their PR is merged.
|
||||
|
||||
From a high level, the ideal life cycle of a pull request looks like the
|
||||
following:
|
||||
|
||||
1. A contributor opens a PR that fixes a specific problem (optimally closing
|
||||
a GitHub `issue <https://github.com/godotengine/godot>`_ or implementing
|
||||
a `proposal <https://github.com/godotengine/godot-proposals>`_).
|
||||
|
||||
2. Other contributors provide feedback on the PR (including reviewing and/or
|
||||
approving the PR, as appropriate).
|
||||
|
||||
3. An engine maintainer reviews the code and provides feedback, requests
|
||||
changes, or approves the pull request, as appropriate.
|
||||
|
||||
4. Another maintainer reviews the code with a focus on code style/clarity and
|
||||
approves it once satisfied.
|
||||
|
||||
5. A team leader or a member of the `production team
|
||||
<https://godotengine.org/teams#production>`_ merges the pull request if
|
||||
satisfied that it has been sufficiently reviewed.
|
||||
|
||||
This document will explain steps 2, 3, 4, and 5 in more detail. For a more
|
||||
detailed explanation of the pull request workflow please see the :ref:`pull
|
||||
request workflow document <doc_pr_workflow>`.
|
||||
|
||||
.. note::
|
||||
In practice these steps may blend together. Oftentimes maintainers will
|
||||
provide comments on code style and code quality at the same time and will
|
||||
approve a pull request for both.
|
||||
|
||||
Typically the first interaction on a pull request will be an engine maintainer
|
||||
assigning tags to the pull request and flagging it for review by someone
|
||||
familiar with that area of code.
|
||||
|
||||
Engine maintainers are folks who are "members" of the Godot project repository
|
||||
on GitHub and/or are listed on the `Teams page <https://godotengine.org/teams>`_
|
||||
on the Godot website. Maintainers are responsible for a given area of the
|
||||
engine. Typically this means they are the people who are given more trust to
|
||||
approve and recommend pull requests for merging.
|
||||
|
||||
Even if you are not a maintainer, you can still help by reviewing code,
|
||||
providing feedback on PRs and testing PRs locally on your machine to confirm
|
||||
that they work as intended. Many of the currently active maintainers started out
|
||||
doing this before they became maintainers.
|
||||
137
pull_request_review/testing.rst
Normal file
137
pull_request_review/testing.rst
Normal file
@@ -0,0 +1,137 @@
|
||||
.. _doc_testing_pull_requests:
|
||||
|
||||
Testing pull requests
|
||||
=====================
|
||||
|
||||
Many people are developing new features or fixing bugs on GitHub.
|
||||
To help with engine development, you may be asked to test those pull requests
|
||||
with a Godot build that includes code from the pull request in question.
|
||||
|
||||
Thanks to GitHub Actions, all `pull requests <https://github.com/godotengine/godot/pulls>`__
|
||||
have continuous builds available. These builds let you try out pull requests
|
||||
without having to compile anything from source.
|
||||
|
||||
Downloading a compiled build
|
||||
----------------------------
|
||||
|
||||
You can download pull request builds from GitHub Actions. Since only signed in
|
||||
users may download builds directly from GitHub Actions, the procedure varies
|
||||
depending on whether you have a GitHub account or not.
|
||||
|
||||
.. note::
|
||||
|
||||
Due to a GitHub Actions limitation, builds are only available for 90 days
|
||||
after the pull request was last updated. If you still wish to try a
|
||||
pull request locally, you can
|
||||
:ref:`compile the pull request branch from source <doc_testing_pull_requests_compile>`
|
||||
instead.
|
||||
|
||||
If you have a GitHub account
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
- Open the pull request page. Click the **Checks** tab near the top of the page:
|
||||
|
||||
.. image:: img/testing_pull_requests_access_checks.webp
|
||||
|
||||
- Click the **Artifacts** dropdown on the right of the page:
|
||||
|
||||
.. image:: img/testing_pull_requests_checks_artifacts.webp
|
||||
|
||||
- In the dropdown, click the artifact's name to download it. Remember to scroll
|
||||
if you cannot see the name of the platform you're looking for:
|
||||
|
||||
.. image:: img/testing_pull_requests_checks_artifacts_list.webp
|
||||
|
||||
- Extract the ZIP archive then run the executable.
|
||||
Note that Windows and macOS binaries are **not** code signed.
|
||||
This means you may have to bypass a security warning before you can run the executable.
|
||||
On Windows, if you frequently test pull request builds, it may be better to disable
|
||||
Windows SmartScreen permanently in the Windows security settings.
|
||||
On macOS, see :ref:`doc_running_on_macos` for instructions on bypassing Gatekeeper.
|
||||
|
||||
If you don't have a GitHub account
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
If you don't have a GitHub account and can't sign up for one,
|
||||
you can use the third-party `nightly.link <https://nightly.link>`__ service
|
||||
to generate a universal download link.
|
||||
|
||||
- Open the pull request page. Click the *fork*'s branch name near the top of the page:
|
||||
|
||||
.. image:: img/testing_pull_requests_access_fork.png
|
||||
|
||||
- Now that you are on the fork's branch page, click the ``.github`` folder at the top of the file list.
|
||||
Then, click on the ``workflows`` folder (which is inside the ``.github`` folder).
|
||||
Click the workflow file for the platform you wish to download artifacts for.
|
||||
*After* clicking on the file (which opens the file view), copy the page URL from your browser's address bar.
|
||||
|
||||
- Open the `nightly.link <https://nightly.link>`__ website and paste the URL you just copied
|
||||
into the text field located below the heading **Paste a GitHub link, get a nightly.link!**.
|
||||
After pasting the URL, click **Get links** on the right.
|
||||
If the format of the URL you pasted is correct, you should be presented
|
||||
with a page like this:
|
||||
|
||||
.. image:: img/testing_pull_requests_nightly_link.png
|
||||
|
||||
- Click the URL of the artifact you wish to download.
|
||||
|
||||
- Extract the ZIP archive then run the executable.
|
||||
Note that Windows and macOS binaries are not code signed.
|
||||
This means you may have to bypass a security warning before you can run the executable.
|
||||
If you frequently test pull request builds, it may be better to disable
|
||||
Windows SmartScreen or `disable macOS Gatekeeper <https://disable-gatekeeper.github.io/>`__ permanently.
|
||||
|
||||
.. _doc_testing_pull_requests_compile:
|
||||
|
||||
Compiling a pull request branch from source
|
||||
-------------------------------------------
|
||||
|
||||
This approach may be needed for pull requests that were last updated more than
|
||||
90 days ago, or to test on platforms and configurations that are not supported
|
||||
by Godot's GitHub Actions setup.
|
||||
|
||||
Downloading a zipped pull request branch
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
- Open the pull request page. Click the *fork*'s branch name near the top of the page:
|
||||
|
||||
.. image:: img/testing_pull_requests_access_fork.png
|
||||
|
||||
- Now that you are on the fork's branch page, click the green **Code** button on the right of the page
|
||||
then choose **Download ZIP** in the dropdown:
|
||||
|
||||
.. image:: img/testing_pull_requests_fork_zip.png
|
||||
|
||||
- Extract the ZIP archive and follow the :ref:`compiling <toc-devel-compiling>` instructions
|
||||
for your operating system.
|
||||
|
||||
Checking out a pull request branch with git
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Alternatively, you can checkout the pull request directly with git:
|
||||
|
||||
- Open the pull request page. Note the pull request *number* (``PR_NUMBER``), and the *branch name*
|
||||
(``BRANCH_NAME``), but without the user name.
|
||||
|
||||
.. image:: img/testing_pull_requests_command_line_checkout.webp
|
||||
|
||||
- Construct the command using this pattern:
|
||||
|
||||
::
|
||||
|
||||
git fetch upstream pull/PR_NUMBER/head:BRANCH_NAME
|
||||
|
||||
So for the pull request above, the actual command will be:
|
||||
|
||||
::
|
||||
|
||||
# Fetch PR branch locally
|
||||
git fetch upstream pull/48734/head:editor_file_dialog_filter_sort
|
||||
|
||||
- Once the pull request finishes downloading, checkout its branch:
|
||||
|
||||
::
|
||||
|
||||
git checkout editor_file_dialog_filter_sort
|
||||
|
||||
- And follow the :ref:`compiling <toc-devel-compiling>` instructions for your operating system.
|
||||
@@ -1,4 +0,0 @@
|
||||
Getting Started
|
||||
===============
|
||||
|
||||
TODO
|
||||
@@ -1,2 +1,36 @@
|
||||
About Issue Triage
|
||||
==================
|
||||
Issue Triage
|
||||
============
|
||||
|
||||
Another great way of contributing to the engine is to test development releases
|
||||
or the development branch and to report issues. It is also helpful to report
|
||||
issues discovered in stable releases, so that they can be fixed in
|
||||
the development branch and in future maintenance releases.
|
||||
|
||||
Testing development versions
|
||||
----------------------------
|
||||
|
||||
To help with the testing, you have several possibilities:
|
||||
|
||||
- Compile the engine from source yourself, following the instructions of the
|
||||
:ref:`Compiling <toc-devel-compiling>` page for your platform.
|
||||
|
||||
- Test official pre-release binaries when they are announced (usually on the
|
||||
blog and other community platforms), such as alpha, beta and release candidate (RC) builds.
|
||||
|
||||
- Test "trusted" unofficial builds of the development branch; just ask
|
||||
community members for reliable providers. Whenever possible, it's best to
|
||||
use official binaries or to compile yourself though, to be sure about the
|
||||
provenance of your binaries.
|
||||
|
||||
As mentioned previously, it is also helpful to keep your eyes peeled for
|
||||
potential bugs that might still be present in the stable releases, especially
|
||||
when using some niche features of the engine which might get less testing by
|
||||
the developers.
|
||||
|
||||
Filing an issue on GitHub
|
||||
-------------------------
|
||||
|
||||
Godot uses `GitHub's issue tracker <https://github.com/godotengine/godot/issues>`_
|
||||
for bug reports. When you start filing a bug report, you’ll be given a form to
|
||||
fill out. Please try to follow it so that all issues are consistent and provide
|
||||
the required information.
|
||||
|
||||
163
triage/bisecting.rst
Normal file
163
triage/bisecting.rst
Normal file
@@ -0,0 +1,163 @@
|
||||
.. _doc_bisecting_regressions:
|
||||
|
||||
Bisecting regressions
|
||||
=====================
|
||||
|
||||
.. highlight:: shell
|
||||
|
||||
Bisecting is a way to find regressions in software. After reporting a bug on the
|
||||
`Godot repository on GitHub <https://github.com/godotengine/godot>`__, you may
|
||||
be asked by a contributor to *bisect* the issue. Bisecting makes it possible for
|
||||
contributors to fix bugs faster, as they can know in advance which commit caused
|
||||
the regression. Your effort will be widely appreciated :)
|
||||
|
||||
The guide below explains how to find a regression by bisecting.
|
||||
|
||||
What is bisecting?
|
||||
------------------
|
||||
|
||||
Godot developers use the `Git <https://git-scm.com/>`__ version control system.
|
||||
In the context of Git, bisecting is the process of performing a manual
|
||||
`binary search <https://en.wikipedia.org/wiki/Binary_search_algorithm>`__
|
||||
to determine when a regression appeared. While it's typically used for bugs,
|
||||
it can also be used to find other kinds of unexpected changes such as
|
||||
performance regressions.
|
||||
|
||||
Using official builds to speed up bisecting
|
||||
-------------------------------------------
|
||||
|
||||
Before using Git's ``bisect`` command, we strongly recommend trying to reproduce
|
||||
the bug with an older (or newer) official release. This greatly reduces the
|
||||
range of commits that potentially need to be built from source and tested.
|
||||
You can find binaries of official releases, as well as alphas, betas,
|
||||
and release candidates `here <https://godotengine.org/download/archive/>`__.
|
||||
|
||||
.. danger::
|
||||
|
||||
Project files may be incompatible between Godot versions.
|
||||
**Make a backup of your project** before starting the bisection process.
|
||||
|
||||
Going from the oldest to the newest build generally reduces the risk of the
|
||||
project not being able to successfully open in the editor, thanks to
|
||||
backwards compatibility. Try to reduce your project to the smallest
|
||||
repeatable example too. The more minimal the project is, the more likely
|
||||
you'll be able to open it without compatibility issues in newer engine
|
||||
versions.
|
||||
|
||||
The Git bisect command
|
||||
----------------------
|
||||
|
||||
If you've found a build that didn't exhibit the bug in the above testing
|
||||
process, you can now start bisecting the regression. The Git version control
|
||||
system offers a built-in command for this: ``git bisect``. This makes the
|
||||
process semi-automated as you only have to build the engine, run it and try to
|
||||
reproduce the bug.
|
||||
|
||||
.. note::
|
||||
|
||||
Before bisecting a regression, you need to set up a build environment to
|
||||
compile Godot from source. To do so, read the
|
||||
:ref:`Compiling <toc-devel-compiling>` page for your target platform.
|
||||
(Compiling Godot from source doesn't require C++ programming knowledge.)
|
||||
|
||||
Note that compiling Godot can take a while on slow hardware (up an hour for
|
||||
each full rebuild on a slow dual-core CPU). This means the full process can
|
||||
take up to several hours. If your hardware is too slow, you may want to stop
|
||||
there and report the results of your "pre-bisecting" on the GitHub issue so
|
||||
another contributor can continue bisecting from there.
|
||||
|
||||
Determine the commit hashes
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
To start bisecting, you must first determine the commit hashes (identifiers) of
|
||||
the "bad" and "good" build. "bad" refers to the build that exhibits the bug,
|
||||
whereas "good" refers to the version that doesn't exhibit the bug.
|
||||
|
||||
You can use either a commit hash (like ``06acfccf8``), the tag of a stable
|
||||
release (like ``4.2.1-stable``), or a branch like ``master``.
|
||||
|
||||
If you're using a pre-release build as the "good" or "bad" build, you can find
|
||||
the commit hash in the Project Manager in the lower-right corner, or in in the
|
||||
**Help > About Godot** dialog in the Godot editor. The version information will
|
||||
look something like ``v4.4.beta3.official [06acfccf8]``, and the commit hash is
|
||||
within the brackets, in this case ``06acfccf8``. You can click on the version
|
||||
information to copy it, including the commit hash.
|
||||
|
||||
Alternately, you can browse the `interactive changelog
|
||||
<https://godotengine.github.io/godot-interactive-changelog/>`__ to find commits
|
||||
for all releases, including development builds. The commits will be listed as a
|
||||
range, like ``commits: a013481b0...06acfccf8``, and the second commit is the one
|
||||
you should use for bisecting. You can also browse the `Godot Archive
|
||||
<https://godotengine.org/download/archive/>`__, and find the commit hash within
|
||||
the release page linked from the **News** button.
|
||||
|
||||
If you're using a stable release as the "good" or "bad" build, you can use the
|
||||
tag of that release directly, such as ``4.2-stable`` or ``4.2.1-stable``. A full
|
||||
list of release tags is available `on GitHub
|
||||
<https://github.com/godotengine/godot/tags>`__, and you can also find the actual
|
||||
commit hash that corresponds to a stable release there.
|
||||
|
||||
To refer to the latest state of the master branch, you can use ``master``
|
||||
instead of a commit hash. Note that unlike tagged releases or snapshot commit
|
||||
hashes, ``master`` is a perpetually moving target.
|
||||
|
||||
Build the engine
|
||||
~~~~~~~~~~~~~~~~
|
||||
|
||||
:ref:`Get Godot's source code using Git <doc_getting_source>`. Once this
|
||||
is done, in the terminal window, use ``cd`` to reach the Godot repository
|
||||
folder and enter the following command:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
# <good commit hash> is hash of the build that works as expected.
|
||||
# <bad commit hash> is hash of the build exhibiting the bug.
|
||||
git bisect start
|
||||
git bisect good <good commit hash>
|
||||
git bisect bad <bad commit hash>
|
||||
|
||||
Compile Godot. This assumes you've set up a build environment:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
scons
|
||||
|
||||
Run the engine
|
||||
~~~~~~~~~~~~~~
|
||||
|
||||
Run the binary located in the ``bin/`` folder and try to reproduce the bug.
|
||||
|
||||
.. note::
|
||||
|
||||
:ref:`Double-check the output file name <doc_introduction_to_the_buildsystem_resulting_binary>`
|
||||
in ``bin/`` to make sure you're actually running the binary you've just compiled.
|
||||
Different Godot versions will output binaries with different names.
|
||||
|
||||
If the build **still** exhibits the bug, run the following command:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
git bisect bad
|
||||
|
||||
If the build **does not** exhibit the bug, run the following command:
|
||||
|
||||
.. code-block:: shell
|
||||
|
||||
git bisect good
|
||||
|
||||
After entering one of the commands above, Git will switch to a different commit.
|
||||
You should now build Godot again, try to reproduce the bug, then enter ``git
|
||||
bisect good`` or ``git bisect bad`` depending on the result. You'll have to
|
||||
repeat this several times. The longer the commit range, the more steps will be
|
||||
required. 5 to 10 steps are usually sufficient to find most regressions; Git
|
||||
will remind you of the number of steps remaining (in the worst case scenario).
|
||||
|
||||
Once you've completed enough steps, Git will display the commit hash where the
|
||||
regression appeared. Write this commit hash as a comment to the GitHub issue
|
||||
you've bisected. This will help in solving the issue. Thanks again for
|
||||
contributing to Godot :)
|
||||
|
||||
.. seealso::
|
||||
|
||||
You can read the full documentation on ``git bisect``
|
||||
`here <https://git-scm.com/docs/git-bisect>`__.
|
||||
Reference in New Issue
Block a user