Page MenuHome
Differential D1357

BGE: bge.constraints.rst: some cleanup
AbandonedPublic
Actions

Authored by Campbell Barton (campbellbarton) on Jun 16 2015, 3:53 PM.

Details

Reviewers
Porteries Tristan (panzergame)
Thomas Szepe (hg1)
Daniel Stokes (kupoman)
Jorge Bernal (lordloki)
Quentin Wenger (Matpi)
Mitchell Stokes (moguri)
Summary

Some cleanup: arguments lists, syntax consistency, default arguments.

Note: there is a misspelling on "treshold" (sic). Sadly it also is the case in the corresponding Python functions, as well as some of the C++ code... :-/ So for now that's not something to be fixed in the doc...

Diff Detail

Repository
rB Blender
Branch
arcpatch-D1357

Event Timeline

Quentin Wenger (Matpi) updated this revision to Diff 4418.Jun 16 2015, 3:53 PM
Quentin Wenger (Matpi) retitled this revision from to BGE: bge.constraints.rst: some cleanup.
Quentin Wenger (Matpi) updated this object.
Quentin Wenger (Matpi) added reviewers: Mitchell Stokes (moguri), Daniel Stokes (kupoman), Campbell Barton (campbellbarton), Porteries Tristan (panzergame), Jorge Bernal (lordloki).
Quentin Wenger (Matpi) added a subscriber: Thomas Szepe (hg1).
Quentin Wenger (Matpi) added a project: Game Engine.Jun 16 2015, 3:55 PM
Quentin Wenger (Matpi) updated this object.Jun 16 2015, 3:58 PM
Quentin Wenger (Matpi) added a reviewer: Thomas Szepe (hg1).
Quentin Wenger (Matpi) removed a subscriber: Thomas Szepe (hg1).
Porteries Tristan (panzergame) requested changes to this revision.Jun 16 2015, 4:09 PM
Porteries Tristan (panzergame) edited edge metadata.
Porteries Tristan (panzergame) added inline comments.
doc/python_api/rst/bge.constraints.rst
19

pivot_X -> pivot_x

40

Why the dot before "(optional)" ?

58

it could be nice to set a constant for this, like : DISABLE_COLLISION_LINKED_BODIES.

66–72

Could you give more info about the file name ? Relative path or absolute ? We can use "//" ?

This revision now requires changes to proceed.Jun 16 2015, 4:09 PM
Quentin Wenger (Matpi) added inline comments.Jun 16 2015, 4:31 PM
doc/python_api/rst/bge.constraints.rst
19

Oops, yes, will change.

40

I find it nicer: optional is something general, not related to the description text imho. It's not the pivot position that is optional, but the parameter that defines it.

Don't know if that makes sense?

I would argument the same way for (readonly) flags.

If you look at bpy.types objects, they are also flagged with the separation symbol before the brackets (but it that case that's a comma, which is not that nice).

Maybe another explaination: the dot is part of the description sentence, not the (optional) flag. For example, what would you consider best:

  • :arg blabla: Don't touch that argument! (optional)

or

  • :arg blabla: Don't touch that argument (optional)!

We can still discuss and change it back if it really doesn't make you happy ;-)

58

But then there would be a need for another constant for the opposite effect... It would actually be much nicer to have a boolean value here...

We can still expose DISABLE_COLLISION_LINKED_BODIES to Python if you wish.

66–72

Dunno, think absolute/relative are possible (FILE* file = fopen(filename,"wb");). Need to check.

Porteries Tristan (panzergame) added inline comments.Jun 16 2015, 6:10 PM
doc/python_api/rst/bge.constraints.rst
66–72

it will nice to add the link about bullet serialization : http://bulletphysics.org/mediawiki-1.5.8/index.php/Bullet_binary_serialization

Thomas Szepe (hg1) added inline comments.Jun 16 2015, 8:15 PM
doc/python_api/rst/bge.constraints.rst
29–38

I moved it out of the arguments (with last patch) and have done it like in setDebugMode, because Sphinx always moved the line "The type of the constraint, one of..." into a new line.
I don't know what looks better. I am also OK with your version.

62

Do you really think it looks better if we only print KX_ConstraintWrapper instead of bge.types.KX_ConstraintWrapper?

Quentin Wenger (Matpi) added inline comments.Jun 16 2015, 9:48 PM
doc/python_api/rst/bge.constraints.rst
29–38

Yes, I am aware of your changes, sorry for acting like I would ignore them...

While I fully understand your point on moving it out, I still have to say I find it a bit difficult to read when constraint types are splitted in two...

This version is actually a bit better than the one before you moved the list, because now the "Parameters" column (pink) is kept in one single block.

About the "one of..." being on a new line, yes I also find it annoying, but still I think I can live with it...

I have to say I'd be happy if this version is OK for you. ;-)

62

I don't really know, actually. I have seen quite many places in the doc where all those prefixes are cut off, and so I overtook that habit.

In this precise case I think it's ok, because:

  1. the hyperlink shows the full version and points to the definition
  2. the prefix KX is quite common and well-known for elements of bge.types.

It would be fully ok for me to keep those prefixes though, BUT then I would like to harmonize that behaviour through the whole BGE api doc (what matters most to me in a doc is consistence, so we simply have to agree with each other on the general settings before).

Quentin Wenger (Matpi) updated this revision to Diff 4426.Jun 17 2015, 12:12 PM
Quentin Wenger (Matpi) edited edge metadata.

Fix signature of createConstraint.

Campbell Barton (campbellbarton) requested changes to this revision.Jun 17 2015, 1:41 PM
Campbell Barton (campbellbarton) edited edge metadata.
Campbell Barton (campbellbarton) added inline comments.
doc/python_api/rst/bge.constraints.rst
267

Wouldn't but the actual values in, just enough to note that they're int bitmasks.

If they change - its a hassle to maintain.

This revision now requires changes to proceed.Jun 17 2015, 1:41 PM
Quentin Wenger (Matpi) updated this revision to Diff 4427.Jun 17 2015, 1:57 PM
Quentin Wenger (Matpi) edited edge metadata.

Delete values of constants, keep types.

Quentin Wenger (Matpi) added a comment.Jun 17 2015, 2:04 PM

@Campbell Barton (campbellbarton): what's your opinion on exposing CCD_CONSTRAINT_DISABLE_LINKED_COLLISION (from source\gameengine\Physics\Bullet\CcdPhysicsEnvironment.cpp) to Python? (for flag argument of createConstraint)

doc/python_api/rst/bge.constraints.rst
267

Makes sense - and the actual values don't matter much.

Changed.

Porteries Tristan (panzergame) added inline comments.Jun 17 2015, 2:45 PM
doc/python_api/rst/bge.constraints.rst
40

Okay

58

You can add 2 module macro:
DISABLE_COLLISION_LINKED_BODIES = 128
ENABLE_COLLISION_LINKED_BODIES = 0

Quentin Wenger (Matpi) updated this revision to Diff 4459.Jun 19 2015, 4:00 PM
Quentin Wenger (Matpi) edited edge metadata.

Add option to save bullet files using local path, update doc for it.

Porteries Tristan (panzergame) accepted this revision.Jun 19 2015, 4:04 PM
Porteries Tristan (panzergame) edited edge metadata.
Porteries Tristan (panzergame) added inline comments.
source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
3013 ↗(On Diff #4459)

FILE *file

Quentin Wenger (Matpi) updated this revision to Diff 4460.Jun 19 2015, 4:10 PM
Quentin Wenger (Matpi) edited edge metadata.

Small syntax enhancement.

Thomas Szepe (hg1) added inline comments.Jun 19 2015, 4:37 PM
source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
3007–3013 ↗(On Diff #4460)

You should not mix documentation changes with functional changes. I think it is better to do that with an extra patch.

Quentin Wenger (Matpi) updated this revision to Diff 4471.Jun 23 2015, 12:13 PM

Revert changes to the export function, update the doc in consequence.

Campbell Barton (campbellbarton) mentioned this in rB1c251d7cb619: Update bge.constraints API doc.Jun 23 2015, 1:42 PM
Campbell Barton (campbellbarton) commandeered this revision.Jun 23 2015, 1:45 PM
Campbell Barton (campbellbarton) edited reviewers, added: Quentin Wenger (Matpi); removed: Campbell Barton (campbellbarton).
This revision is now accepted and ready to land.Jun 23 2015, 1:45 PM
Campbell Barton (campbellbarton) abandoned this revision.EditedJun 23 2015, 1:50 PM

Applied, closing... some notes...

  • No need to list int, all over, these are just to pass into functions.
  • Just mention absolute path is enough for saving bullet files. (no need to explain what that is...)
  • removed Functions header, we may want to have headers in these files but this depends on the header in bge.constraints.py, Id rather not make formatting depend on headers in examples. Better have 2x headers one for eg:
| Examples
| ------------
| 
| .. literalinclude:: ....
| 
| Functions
| -------------
    
...

Revision Contents
Changeset List

PathSize
doc/
python_api/
rst/
173 lines

Diff 4471

View Options

doc/python_api/rst/bge.constraints.rst

Loading...