Skip to content

Commit c1101c3

Browse files
akien-mgaakien-mga
authored
Merge pull request #9158 from AThousandShips/compat_doc
Document how to handle compatibility breakages
2 parents 2b797d6 + 3fafd46 commit c1101c3

File tree

3 files changed

+143
-2
lines changed

3 files changed

+143
-2
lines changed

‎about/release_policy.rst‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,5 +275,4 @@ compatibility-breaking changes of this kind.
275275
optional parameter), a GDExtension compatibility method must be created.
276276
This ensures that existing GDExtensions continue to work across patch and
277277
minor releases, so that users don't have to recompile them.
278-
See `pull request #76446 <https://github.com/godotengine/godot/pull/76446>`_
279-
for more information.
278+
See :ref:`doc_handling_compatibility_breakages` for more information.

‎contributing/development/handling_compatibility_breakages.rst‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
.. _doc_handling_compatibility_breakages:
2+
3+
Handling compatibility breakages
4+
================================
5+
6+
.. TODO: Elaborate on types of compatibility and procedure.
7+
8+
So you've added a new parameter to a method, changed the return type,
9+
changed the type of a parameter, or changed its default value,
10+
and now the automated testing is complaining about compatibility breakages?
11+
12+
Breaking compatibility should be avoided, but when necessary there are systems in place
13+
to handle this in a way that makes the transition as smooth as possible.
14+
15+
A practical example
16+
-------------------
17+
18+
.. TODO: Add example that showcases more details like original default arguments etc.
19+
20+
These changes are taken from `pull request #88047 <https://github.com/godotengine/godot/pull/88047>`_, which added
21+
new pathing options to ``AStarGrid2D`` and other AStar classes.
22+
Among other changes, these methods were modified in ``core/math/a_star_grid_2d.h``:
23+
24+
.. code-block:: cpp
25+
26+
Vector<Vector2> get_point_path(const Vector2i &p_from, const Vector2i &p_to);
27+
TypedArray<Vector2i> get_id_path(const Vector2i &p_from, const Vector2i &p_to);
28+
29+
To:
30+
31+
.. code-block:: cpp
32+
33+
Vector<Vector2> get_point_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false);
34+
TypedArray<Vector2i> get_id_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false);
35+
36+
This meant adding new compatibility method bindings to the file, which should be in the ``protected`` section of
37+
the code, usually placed next to ``_bind_methods()``:
38+
39+
.. code-block:: cpp
40+
41+
#ifndef DISABLE_DEPRECATED
42+
TypedArray<Vector2i> _get_id_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to);
43+
Vector<Vector2> _get_point_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to);
44+
static void _bind_compatibility_methods();
45+
#endif
46+
47+
They should start with a ``_`` to indicate that they are internal, and end with ``_bind_compat_`` followed by the PR number
48+
that introduced the change (``88047`` in this example). These compatibility methods need to be implemented in a dedicated file,
49+
like ``core/math/a_star_grid_2d.compat.inc`` in this case:
50+
51+
.. code-block:: cpp
52+
53+
/**************************************************************************/
54+
/* a_star_grid_2d.compat.inc */
55+
/**************************************************************************/
56+
/* This file is part of: */
57+
/* GODOT ENGINE */
58+
/* https://godotengine.org */
59+
/**************************************************************************/
60+
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
61+
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
62+
/* */
63+
/* Permission is hereby granted, free of charge, to any person obtaining */
64+
/* a copy of this software and associated documentation files (the */
65+
/* "Software"), to deal in the Software without restriction, including */
66+
/* without limitation the rights to use, copy, modify, merge, publish, */
67+
/* distribute, sublicense, and/or sell copies of the Software, and to */
68+
/* permit persons to whom the Software is furnished to do so, subject to */
69+
/* the following conditions: */
70+
/* */
71+
/* The above copyright notice and this permission notice shall be */
72+
/* included in all copies or substantial portions of the Software. */
73+
/* */
74+
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
75+
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
76+
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
77+
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
78+
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
79+
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
80+
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
81+
/**************************************************************************/
82+
83+
#ifndef DISABLE_DEPRECATED
84+
85+
#include "core/variant/typed_array.h"
86+
87+
TypedArray<Vector2i> AStarGrid2D::_get_id_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) {
88+
return get_id_path(p_from_id, p_to_id, false);
89+
}
90+
91+
Vector<Vector2> AStarGrid2D::_get_point_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) {
92+
return get_point_path(p_from_id, p_to_id, false);
93+
}
94+
95+
void AStarGrid2D::_bind_compatibility_methods() {
96+
ClassDB::bind_compatibility_method(D_METHOD("get_id_path", "from_id", "to_id"), &AStarGrid2D::_get_id_path_bind_compat_88047);
97+
ClassDB::bind_compatibility_method(D_METHOD("get_point_path", "from_id", "to_id"), &AStarGrid2D::_get_point_path_bind_compat_88047);
98+
}
99+
100+
#endif // DISABLE_DEPRECATED
101+
102+
Unless the change in compatibility is complex, the compatibility method should simply call the modified method directly,
103+
instead of duplicating that method. Make sure to match the default arguments for that method (in the example above this would be ``false``).
104+
105+
This file should always be placed next to the original file, and have ``.compat.inc`` at the end instead of ``.cpp`` or ``.h``.
106+
Next, this should be included in the ``.cpp`` file we're adding compatibility methods to, so ``core/math/a_star_grid_2d.cpp``:
107+
108+
.. code-block:: cpp
109+
110+
#include "a_star_grid_2d.h"
111+
#include "a_star_grid_2d.compat.inc"
112+
113+
#include "core/variant/typed_array.h"
114+
115+
And finally, the changes reported by the API validation step should be added to the relevant validation file. Because this was
116+
done during the development of 4.3, this would be ``misc/extension_api_validation/4.2-stable.expected`` (including changes not shown in
117+
this example):
118+
119+
.. code-block:: text
120+
121+
GH-88047
122+
--------
123+
Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
124+
Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
125+
Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
126+
Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
127+
Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3.
128+
Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3.
129+
130+
Added optional "allow_partial_path" argument to get_id_path and get_point_path methods in AStar classes.
131+
Compatibility methods registered.
132+
133+
The instructions for how to add to that file are at the top of the file itself.
134+
135+
If you get a "Hash changed" error for a method, it means that the compatibility binding is missing or incorrect.
136+
Such lines shouldn't be added to the ``.expected`` file, but fixed by binding the proper compatibility method.
137+
138+
And that's it! You might run into a bit more complicated cases, like rearranging arguments,
139+
changing return types, etc., but this covers the basic on how to use this system.
140+
141+
For more information, see `pull request #76446 <https://github.com/godotengine/godot/pull/76446>`_.

‎contributing/development/index.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ especially if you're new to using Git or GitHub.
1919
best_practices_for_engine_contributors
2020
code_style_guidelines
2121
cpp_usage_guidelines
22+
handling_compatibility_breakages
2223

2324
Buildsystem and work environment
2425
--------------------------------

0 commit comments

Comments
 (0)