Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Sync classref with current source #43310

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Nov 4, 2020

Includes various changes triggered by the refactoring of method bindings in #42780.


Draft as some of the changes might be bugs that need to be addressed before we make them harder to spot.

@@ -195,29 +195,6 @@
[/codeblocks]
</description>
</method>
<method name="from_hsv">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing binding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually commented out:

// FIXME: Color is immutable, need to probably find a way to do this via constructor
//ADDFUNC4R(COLOR, COLOR, Color, from_hsv, FLOAT, "h", FLOAT, "s", FLOAT, "v", FLOAT, "a", varray(1.0));

doc/classes/Geometry2D.xml Outdated Show resolved Hide resolved
@@ -20,9 +20,9 @@
</description>
</method>
<method name="append">
<return type="void">
<return type="bool">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come append and push_back now return bool? It seems to match what is defined in core/vector.h but it returns true on failure and false on error, is that a weird Unix-style error handling?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an intended change but it comes from the C++ API. We now have to make sure that the C++ API matches what we want to expose to the scripting API.

Here I'm not sure there's any C++ code that relies on those return values, so they could possibly be changed to void to preserve the previous scripting API.

Comment on lines -135 to -154
<method name="set_axis_angle">
<return type="void">
</return>
<argument index="0" name="axis" type="Vector3">
</argument>
<argument index="1" name="angle" type="float">
</argument>
<description>
Sets the quaternion to a rotation which rotates around axis by the specified angle, in radians. The axis must be a normalized vector.
</description>
</method>
<method name="set_euler">
<return type="void">
</return>
<argument index="0" name="euler" type="Vector3">
</argument>
<description>
Sets the quaternion to a rotation specified by Euler angles (in the YXZ convention: when decomposing, first Z, then X, and Y last), given in the vector format as (X angle, Y angle, Z angle).
</description>
</method>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing bindings, or removed on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out in variant_call.cpp:

// FIXME: Quat is atomic, this should be done via construcror
//ADDFUNC1(QUAT, NIL, Quat, set_euler, VECTOR3, "euler", varray());
//ADDFUNC2(QUAT, NIL, Quat, set_axis_angle, VECTOR3, "axis", FLOAT, "angle", varray());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that #30744 finally got resolved in core :)

doc/classes/Rect2i.xml Outdated Show resolved Hide resolved
@@ -397,17 +405,6 @@
Returns [code]true[/code] if the string ends with the given string.
</description>
</method>
<method name="erase">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing binding, or removed on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out in variant_call.cpp:

// FIXME: String needs to be immutable when binding
//bind_method(String, erase, sarray("position", "chars"), varray());

@@ -512,20 +512,6 @@
[/codeblock]
</description>
</method>
<method name="humanize_size">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing binding, or removed on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out in variant_call.cpp:

// FIXME: Static function, not sure how to bind
//bind_method(String, humanize_size, sarray("size"), varray());

doc/classes/Vector2.xml Outdated Show resolved Hide resolved
doc/classes/String.xml Outdated Show resolved Hide resolved
@akien-mga akien-mga marked this pull request as ready for review November 4, 2020 13:52
@akien-mga akien-mga requested a review from a team as a code owner November 4, 2020 13:52
Includes various changes triggered by the refactoring of method bindings.
@akien-mga
Copy link
Member Author

Updated with classref changes from #43297 (+ added missing bindings).

@akien-mga
Copy link
Member Author

I opened #43311 to keep track of the lost method bindings.

@akien-mga akien-mga merged commit 8e13b93 into godotengine:master Nov 4, 2020
@akien-mga akien-mga deleted the doc-classref-sync branch November 4, 2020 15:20
jynus added a commit to jynus/godot that referenced this pull request Jul 24, 2022
On godotengine#43310, class reference was automatically updated from source,
causing xml documentation to disagree with parameter naming
description on Plane.intersects_segment().

Weirdly, it also changed the parameter for Plane.is_point_over()
from point to plane, when only the first has sense (and it is
defined on math.Plane as "const Vector3 &p_point"). Manual
mistake?

* Update begin/end to from/to on Plane.intersects_segment(...)
  docs description to match source
* Update Plane bindings to use points instread of plane for
  is_point_over(...)
* Change Plane.is_point_over(plane) to Plane.is_point_over(point)
  AND its description on docs

Fixes godotengine/godot-docs#5976
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants