Upbge: Feature Request: some methods of KX_GameObject take no keyword arguments

[vlad0337187]

Hello.
I was faced with this problem many times in BGE, now I was faced with it and in UPBGE.
As here is present bugtracker, so I send here bugreport.

Error: methods, which were referenced as those, which can take keyword arguments, cannot take such keyword arguments.

Example of method with such error:
In API Wiki is written:

sendMessage(subject, body="", to="")
Sends a message. Parameters:
subject (string) – The subject of the message
body (string) – The body of the message (optional)
to (string) – The name of the object to send the message to (optional)

But when I run code with "obj.sendMessage('trade1', body='', to='trader')", than I get error:

TypeError: sendMessage() takes no keyword arguments

_(in Python, arguments in format " arg='value' " mean named arguments when we're calling function; and mean default values when we create function. In API Wiki there are examples of calling functions, not creating, so it mean namely keyword arguments)_

Example of method without such error:
In API Wiki is written:

rayCast(objto, objfrom, dist, prop, face, xray, poly, mask)

As it was referenced, that he cannot take named arguments, so it cannot do this. No errors.

Possible decisions:

• 
  
  1. Remove named arguments from description in Python API Wiki:
  
  

sendMessage(subject, body, to)
Sends a message. Parameters:
subject (string) – The subject of the message
body (string) – The body of the message (optional)
to (string) – The name of the object to send the message to (optional)

_benefits_: - there is no error in descripton\code.
_disadvantages_: - functionality and clarity reduce
(with named arguments we can pass one or more certain arguments: ".sendMessage('dfd', to='Cube')", without them we need to pass all values: .sendMessage('dfd', '', 'Cube'). This is especially noticeable when function takes many arguments, like .playAction() does (it takes 11 arguments, 8 from them - keyword))

• 
  
  1. Add possibility to pass named arguments to methods:
  
  

Cube3.sendMessage('take', to='Cube4')

_benefits_: - there is no error in descripton\code; - functionality and clarity will not decrease.
_disadvantages_: - extra work from developers.

Known methods with such error:
https://github.com/UPBGE/blender/issues/237#issuecomment-250989457
(thanks to @sdfgeoff for his work)

(I'll continue this list when I'll test other methods)

Best regards,
Vladislav.

[panzergame]

@vlad1777d : What documentation style do you propose to write the default value of a default argument ?

[vlad0337187]

@panzergame , I propose to write them below in the descriptions of each parameter:

sendMessage(subject, body, to)
Sends a message. Parameters:
subject (string) (default='value') – The subject of the message
body (string) (default='') – The body of the message (optional)
to (string) – The name of the object to send the message to (optional)

But if you want to write them in one line with method call, than you can add additional scopes:

sendMessage(subject [default='value'], body, to)
applyImpulse(point, impulse, local=False [default=False])

Or to use comments:

sendMessage(subject, body, to) # defaults: subject='value', body=''
applyImpulse(point, impulse, local=False) # defaults: local=False

Aha, I understood. Maybe there were mean default values.
But that syntax goes against with Python's syntax, where such combinations mean named arguments. Especially when there are empty quotes, this looks like description: this is the named argument, pass here your string.
Default values in Python are written the same way, but only when you define the function.

[sdfgeoff]

I would say edit blender to make them named arguments if you can. It won't break existing modules or scripts. I'd like to be able to run:
sendMessage(subject, body="Body", to="Cube")
and have it run correctly. Perhaps even for functions where this isn't currently the case. For example a rayCast has many optional parameters, this would allow users to specifying only the parameters you actually care about.

I'd like to be able to do:
rayCast(objto, objfrom, poly=True)
As opposd to having to specify values for everything to tweak the parameter at the end:
rayCast(objto, objfrom, dist=?, prop=?, face=?, xray=?, poly=True)

[youle31]

I agree for raycast: we should use PyArg_ParseTupleAndKeywords method instead of PyArg_ParseTuple(args,"O|Ofsiiii:rayCast", &pyto, &pyfrom, &dist, &propName, &face, &xray, &poly, &mask)) {

For information:

For python method we implement, we often use PyArg_ParseTuple.

The "|" means that the args which are after this symbol are optional.

When we use PyArg_ParseTupleAndKeywords, the method looks like: http://pasteall.org/82821/c

This allows to use keywords for arguments.

As this is not a bug, I think we can close the issue. @vlad1777d : You can formulate a request to tell us in which methods you want we use keywords.

[vlad0337187]

@youle31 if it disturbs you - you can close it.

I thought that it would be better to close it when it'll be implemented (I could follow it), but this is my own thoughts, I understand, that when there are many opened issues, it can make searching through them less convenient, so if it obstructs you - than of course, close it.

I'll write a little bit later needed methods, just after a little rest.

@sdfgeoff , you can write methods that you want in your message and I'll copy them to list in my first message.

[sdfgeoff]

@vlad1777d
Youle just means it's not a bug. This tracker is for bugs rather than feature/improvement requests.

That said, I have compiled a list of things with optional arguments that could be considered as candidates for changing. I went through the entire bge subsection, but have not been through mathutils, aud, gpu etc. It is likely this is an ehxaustive list for the following sections of API:

• Game Types (bge.types)
• Game Logic (bge.logic)
• Rasterizer (bge.render)
• Video Texture (bge.texture)
• Game Keys (bge.events)
• Physics Constraints (bge.constraints)
• Application Data (bge.app)

BL_Shader:
setUniformMatrix3 has transpose as an optional argument
setUniformMatrix4
KX_2DFilterManager
addFilter has fragmentShader as an optional argument
KX_Camera
getScreenRay(x, y, dist=inf, property=None)
KX_MeshProxy
transformUV(matid, matrix, uv_index=-1, uv_index_from=-1)
KX_Scene
addObject(object, reference, time=0.0)
KX_GameObject
setVisible has recursive as an optional argument
setOcclusion has recursive as an optional argument
alignAxisToVect(vect, axis=2, factor=1.0)
applyMovement(movement, local=False)
applyRotation(rotation, local=False)
applyForce(force, local=False)
applyTorque(torque, local=False)
getLinearVelocity(local=False) - Possible candidate for deprecation?
setLinearVelocity(velocity, local=False) - Possible candidate for deprecation?
getAngularVelocity(local=False) - Possible candidate for deprecation?
setAngularVelocity(velocity, local=False) - Possible candidate for deprecation?
getVelocity(point=(0, 0, 0))
applyImpulse(point, impulse, local=False)
setParent(parent, compound=True, ghost=True)
rayCastTo has "dist" and "prop" as optional arguments
rayCast has "dist, prop, face, xray, poly, mask" all optional
sendMessage(subject, body="", to="")
reinstancePhysicsMesh has two mutually exclusive optional arguments
playAction(name, start_frame, end_frame, layer=0, priority=0, blendin=0, play_mode=KX_ACTION_MODE_PLAY, layer_weight=0.0, ipo_flags=0, speed=1.0, blend_mode=KX_ACTION_BLEND_BLEND)
stopAction(layer=0)
getActionFrame(layer=0)
getActionName(layer=0)
setActionFrame(frame, layer=0)
isPlayingAction(layer=0)
addDebugProperty(name, debug = True)
The last 6 should possible have the documentation changed as the arguments don't look optional at first glance.

bge.logic
LibLoad(blend, type, data, load_actions=False, verbose=False, load_scripts=True, async=False)
addScene(name, overlay=1)
sendMessage(subject, body="", to="", message_from="")
bge.logic.getBlendFileList(path = "//")

bge.texture
VideoFFmpeg(file, capture=-1, rate=25.0, width=0, height=0)
ImageFFmpeg.refresh(buffer=None, format="RGBA")
ImageFFmpeg.reload(newname=None)
ImageBuff(width, height, color=0, scale=False)
ImageBuff.plot(imageBuffer, width, height, positionX, positionY, mode=IMB_BLEND_COPY)
ImageMirror(scene, observer, mirror, material=0, width, height, samples) - optional argument in the middle?!
ImageMirror.refresh(buffer=None, format="RGBA")
ImageMix.refresh(buffer=None, format="RGBA")
ImageRender.refresh(buffer, format="RGBA")
ImageViewport.refresh(buffer=None, format="RGBA")
VideoDeckLink(format, capture=0)

Texture(gameObj, materialID=0, textureID=0, textureObj=None)
Texture.refresh(refresh_source, timestamp=-1.0)
DeckLink(cardIdx=0, format="")

imageToArray(image, mode) - mode is optional

[panzergame]

@vlad1777d : i agree to use keywords for python functions like rayCast which have more than 2 optional arguments, else there's a lot of functions using maximum two optional arguments, which is, as my opinion, not needed to use keywords.

[sdfgeoff]

Where there is only one optional argument there (perhaps) isn't as much point. In some cases it may be "nice" to anyway, or at least change the documentation so it doesn't look like they're keyword arguments.

If someone wants to spend half an hour and teach me how to change these, I'm happy to be a monkey and spend an hour or two changing lots of them. (I do have C++ experience, but not with the BGE code-base or python bindings)

I will point out a few things from the previous list:

_bge.texture.ImageMirror(scene, observer, mirror, material=0, width, height, samples)_
How do we have an optional argument in the middle?
_reinstancePhysicsMesh_
has two mutually exclusive optional arguments, to use the second one you have to pass the first as "None"

[youle31]

In ImageRender.cpp, there is the python method for ImageMirror: http://pasteall.org/82955/c
So, there is an error in the doc. The doc is in \upbgeSources\doc\python_api\rst\bge_types

If you want to do a patch for upbge, we can help you on #upbgecoders. I'm working on Visual Studio but maybe someone who has qtcreator can help.

In most cases, you type the name of the method you want to modify in the qtcreator search bar, you take example on the methods that already use PyArg_ParseTupleAndKeywords and you modify the code of the method according to. Then you modify the documentation taking example on what is already existing and not wrong.

Once you did all the modifs, to save your patch, you have several options with git.

• You can save it into a branch
• You can create a .diff file -> you open a terminal in upbgeSources, you type:
  git diff HEAD > myPatch.diff

To reset all your modifs, you type:
git reset --hard (warning: it will delete all the modifs you made)

For any question you can ask on #upbgecoders to panzergame, TwisterGE or me when we are connected (very often)

EDIT: I think this patch would be more useful to commit in BF Blender then imported in upbge.

[sdfgeoff]

I'm working on Visual Studio but maybe someone who has qtcreator can help.

Eh, I haven't used an IDE in years. Text editors and terminals are the way I work currently. Maybe someday someone will convert me back to the land of 'features.'

....you take example on the methods that already use PyArg_ParseTupleAndKeywords....

Thanks for the name, I ran a grep for it and found some examples. I've now converted my first function.

Should I:

• Do them all and then do one massive diff? - Seems like it would be massive given the list above
• Do each fuction individually? - Seems like a lot of overhead
• Do one diff per file? Some files only have one change, some have many,

Submitting diffs to the blender diff thing is scary, particularly when this has been discussed here rather than with the 'actual' blender developers. I'm unsure about if this should be submitted to blender or UPBGE.
I'll try and jump on IRC in a couple hours and ask some more questions then.

Notes to self about the conversion process:

• Change function definition to KX_PYMETHODDEF_DOC
• In function insert a line listing all the arguments like: static const char *kwlist[] = {"x", "y", "dist", "propName", NULL};
• In function change to if (!PyArg_ParseTupleAndKeywords(args, kwds, string_of_types, const_cast<char**>(kwlist), &var, &var, &var)
• Change header file declaration to KX_PYMETHOD_DOC
• Change reference in object::Methods to KX_PYMETHODTABLE_KEYWORDS
• Type make and cross your fingers....

[sdfgeoff]

Heh, seems like some documentation is incorrect as to what properties are actually optional!

Darn, it also seems that some of the code uses a different (older?) method for settings methods. This may end up being larger than I thought.
Some objects define methods as KX_PYMETHODTABLE_O(KX_Camera, boxInsideFrustum)
And others use {"getVertex", (PyCFunction) KX_MeshProxy::sPyGetVertex, METH_VARARGS},
I'm guessing that the intention is to switch to using the first method, as it looks visually cleaner.

I hate to combine two tasks, but since I'm touching many of the functions anyway, should I convert them to the first style of definition?

[youle31]

@sdfgeoff : Maybe it's a discussion to have with @panzergame as he is an official BF Blender dev and as he's often on #upbgecoders.

[sdfgeoff]

Started work on it. I'm changing a few other things as I go though - only things to do with the python interface (such as error messages on bad arguments). It's going to take a while.

[TryExceptElse]

I ran some tests, seeing if adding kwargs would have a performance hit, and it seems that parsing kwargs as well as conventional args adds ~0.1us to each method call on my machine (fairly modern as of 2017), so in a simple test scene, a rayCast on my machine goes from 2.4us to 2.5us.

When called using key word arguments, the time goes up to 3.7us or so.

While this seems ok to me, I think there might be people upset out there that their code runs slower.

Thoughts?

[BluePrintRandom]

yeah, I personally would like the raw speed over this function,

except a few corner cases (like set object property dictionary)
but there may already be other commands for this*

On Sep 17, 2017 8:45 AM, "TryExceptElse" [e-mail] wrote:

I ran some tests, seeing if adding kwargs would have a performance hit,
and it seems that parsing kwargs as well as conventional args adds ~0.1us
to each method call on my machine (fairly modern as of 2017), so in a
simple test scene, a rayCast on my machine goes from 2.4us to 2.5us.

When called using key word arguments, the time goes up to 3.7us or so.

While this seems ok to me, I think there might be people upset out there
that their code runs slower.

Thoughts?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/UPBGE/blender/issues/237#issuecomment-330057664, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AG25WVhEZPiE6lP3QBHoP88P1WdXA1SLks5sjT6NgaJpZM4KLxwj
.