Cleanup of Python API - Naming inconsistency, deprecated content, etc

[joelgomes1994]

Probably this is already planned to be done, but it's good to be documented as an issue, in my point of view. Since a lot of people worked in BGE in the past years and some of them didn't followed a single rule of writing, the API became pretty messy because the lack of naming consistency of variables and functions. The presence of deprecated content contributes to this mess too. I will try to list all the inconsistences (none of them are right or wrong, but are written in different ways, without a standard).

The "correct" naming of function and variable identifiers is assumed to be in the format "nameName", since most of the API is using the format. Ex: bge.constraints.setGravity()

Use of underlines to separate words (and all lower case) _* I really like this style, namename is a lot more readable than nameName. But BGE is not Godot :smile:

• BL_ArmatureBone (inherit_scale, bbone_segments, arm_head, arm_tail, arm_mat, bone_mat)
• BL_ArmatureChannel (has_ik, ik_dof_x/y/z, ik_limit_x/y/z, ik_rot_control, ik_lin_control, rotation_quaternion, rotation_euler, rotation_mode, channel_matrix, pose_matrix, pose_head, pose_tail, ik_min_x/y/z, ik_max_x/y/z, ik_stiffness_x/y/z, ik_stretch, ik_rot_weight, ik_lin_weight, joint_rotation)
• BL_ArmatureConstraint (lin_error, rot_error, ik_weight, ik_type, ik_flag, ik_dist, ik_mode)
• CListValue (from_id)
• KX_BlenderMaterial (material_index)
• KX_Camera (ortho_scale, shift_x, shift_y, frustum_culling, projection_matrix, modelview_matrix, camera_to_world, world_to_camera)
• KX_ConstraintWrapper (constraint_id, constraint_type)
• KX_GameObject (record_animation)
• KX_LightObject (lin_attenuation, quad_attenuation)
• KX_MouseActuator (use_axis_x/y, reset_x/y, object_axis, local_x/y, limit_x/y)
• KX_PolyProxy (material_name, texture_name, material_id)
• KX_Scene (active_camera, activity_culling, activity_culling_radius, dbvt_culling, pre_draw, post_draw, pre_draw_setup)
• KX_SoundActuator (volume_maximum, volume_minimum, distance_reference, distance_maximum, cone_angle_inner/outer, cone_volume_outer)
• SCA_ISensor (pos_ticks, neg_ticks)
• SCA_PythonKeyboard (active_events)
• SCA_PythonMouse (active_events)
• bge.app (version_string, version_char, has_texture_ffmpeg, has_joystick, has_physics)

Use of uper case in the first character of each word (classes not included)

• bge.logic (LibLoad, LibNew, LibFree, LibList, NextFrame, PrintGLInfo, PrintMemInfo)
• bge.events (EventToString, EventToCharacter)

Deprecated content

• KX_GameObject (position, orientation, scaling)
• SCA_ISensor (frequency)
• bge.render (enableVisibility, setBackgroundColor)
• bge.events (RETKEY)

[youle31]

Thanks for this remarks. Unfortunately, we can't remove existing API (edit: not from the API, from the code) even if the norm is to use camelCase now. Why? Because of backward compatibility.

So for example, we can't remove scene.active_camera because a lot of .blend already use this function. What we can do is add an alias -> scene.activeCamera which will use the same function. We can eventually remove active_camera from the API and keep only activeCamera...

Same for deprecated functions, we can't remove object.orientation because a lot of .blend already use that. In some cases we add a deprecation warning.

[panzergame]

About the names issue, i though of a system generating the both names, a kind of macro where you pass a list of words. e.g:

KX_PYATTRIBUTE_INT_RO({"lin", "attenuation"}, 0, 10, pyattr_get_lin_attenuation);

Which should give: lin_attenuation and linAttenuation

[sdfgeoff]

I think that at some points, you have to break backward compatibility. Like the change between python2 and python3 at blender 2.49 -> 2.5 I think the naming at the moment is a significant drawback as it means I have to refer to the API a lot even when I know the name of what I want to do.

I'd say that when you hit a significant milestone of some sort, just up and declare that you've changed it in really big letters. Better yet, provide a python script/addon that changes all the capitalization. Maybe a few versions before hand, deprecate all the ones that are about to change and make them spam the console in some obvious way.

[panzergame]

The list of non camel case attributes is:

KX_MouseFocusSensor.neg_ticks
KX_MouseFocusSensor.pos_ticks
SCA_MouseSensor.neg_ticks
SCA_MouseSensor.pos_ticks
KX_NetworkMessageSensor.neg_ticks
KX_NetworkMessageSensor.pos_ticks
KX_CollisionSensor.neg_ticks
KX_CollisionSensor.pos_ticks
SCA_ISensor.neg_ticks
SCA_ISensor.pos_ticks
KX_NearSensor.neg_ticks
KX_NearSensor.pos_ticks
KX_ArmatureSensor.neg_ticks
KX_ArmatureSensor.pos_ticks
SCA_JoystickSensor.neg_ticks
SCA_JoystickSensor.pos_ticks
KX_Camera.camera_to_world
KX_Camera.frustum_culling
KX_Camera.modelview_matrix
KX_Camera.ortho_scale
KX_Camera.projection_matrix
KX_Camera.shift_x
KX_Camera.shift_y
KX_Camera.world_to_camera
SCA_PythonMouse.active_events
BL_ArmatureChannel.channel_matrix
BL_ArmatureChannel.has_ik
BL_ArmatureChannel.ik_dof_x
BL_ArmatureChannel.ik_dof_y
BL_ArmatureChannel.ik_dof_z
BL_ArmatureChannel.ik_limit_x
BL_ArmatureChannel.ik_limit_y
BL_ArmatureChannel.ik_limit_z
BL_ArmatureChannel.ik_lin_control
BL_ArmatureChannel.ik_lin_weight
BL_ArmatureChannel.ik_max_x
BL_ArmatureChannel.ik_max_y
BL_ArmatureChannel.ik_max_z
BL_ArmatureChannel.ik_min_x
BL_ArmatureChannel.ik_min_y
BL_ArmatureChannel.ik_min_z
BL_ArmatureChannel.ik_rot_control
BL_ArmatureChannel.ik_rot_weight
BL_ArmatureChannel.ik_stiffness_x
BL_ArmatureChannel.ik_stiffness_y
BL_ArmatureChannel.ik_stiffness_z
BL_ArmatureChannel.ik_stretch
BL_ArmatureChannel.joint_rotation
BL_ArmatureChannel.pose_head
BL_ArmatureChannel.pose_matrix
BL_ArmatureChannel.pose_tail
BL_ArmatureChannel.rotation_euler
BL_ArmatureChannel.rotation_mode
BL_ArmatureChannel.rotation_quaternion
SCA_KeyboardSensor.neg_ticks
SCA_KeyboardSensor.pos_ticks
SCA_PropertySensor.neg_ticks
SCA_PropertySensor.pos_ticks
KX_LightObject.lin_attenuation
KX_LightObject.quad_attenuation
KX_SoundActuator.cone_angle_inner
KX_SoundActuator.cone_angle_outer
KX_SoundActuator.cone_volume_outer
KX_SoundActuator.distance_maximum
KX_SoundActuator.distance_reference
KX_SoundActuator.volume_maximum
KX_SoundActuator.volume_minimum
KX_MouseActuator.limit_x
KX_MouseActuator.limit_y
KX_MouseActuator.local_x
KX_MouseActuator.local_y
KX_MouseActuator.object_axis
KX_MouseActuator.reset_x
KX_MouseActuator.reset_y
KX_MouseActuator.use_axis_x
KX_MouseActuator.use_axis_y
SCA_AlwaysSensor.neg_ticks
SCA_AlwaysSensor.pos_ticks
BL_ArmatureBone.arm_head
BL_ArmatureBone.arm_mat
BL_ArmatureBone.arm_tail
BL_ArmatureBone.bbone_segments
BL_ArmatureBone.bone_mat
BL_ArmatureBone.inherit_scale
KX_MovementSensor.neg_ticks
KX_MovementSensor.pos_ticks
KX_RadarSensor.neg_ticks
KX_RadarSensor.pos_ticks
SCA_PythonKeyboard.active_events
SCA_RandomSensor.neg_ticks
SCA_RandomSensor.pos_ticks
KX_Scene.active_camera
KX_Scene.activity_culling
KX_Scene.activity_culling_radius
KX_Scene.dbvt_culling
KX_Scene.post_draw
KX_Scene.pre_draw
KX_Scene.pre_draw_setup
CListValue.from_id
KX_PolyProxy.material_id
KX_PolyProxy.material_name
KX_PolyProxy.texture_name
KX_RaySensor.neg_ticks
KX_RaySensor.pos_ticks
SCA_ActuatorSensor.neg_ticks
SCA_ActuatorSensor.pos_ticks
KX_ConstraintWrapper.constraint_id
KX_ConstraintWrapper.constraint_type
SCA_DelaySensor.neg_ticks
SCA_DelaySensor.pos_ticks
BL_ArmatureConstraint.ik_dist
BL_ArmatureConstraint.ik_flag
BL_ArmatureConstraint.ik_mode
BL_ArmatureConstraint.ik_type
BL_ArmatureConstraint.ik_weight
BL_ArmatureConstraint.lin_error
BL_ArmatureConstraint.rot_error

[organicpencil]

^ Yes please. Fixing code is trivial. Personally I'm less concerned about API breakage than say, incompatible materials. I would love to see UPBGE take this direction.

[youle31]

Why break backward compatibility whereas we Just have to make aliases? Or what panzergame proposed: For all attr, make camel case and underscore available

[DCubix]

@youle31 Why work twice for this? I would rewrite everything If I could

[youle31]

Work Twice? It's just a line of code to add: e.g: KX_PYATTRIBUTE_RW_FUNCTION("active_camera", KX_Scene, pyattr_get_active_camera, pyattr_set_active_camera), KX_PYATTRIBUTE_RW_FUNCTION("activeCamera", KX_Scene, pyattr_get_active_camera, pyattr_set_active_camera),

not that hard ... And we can automate this as panzergame suggested.

[organicpencil]

So long as those old methods are deprecated and (eventually) removed. It wouldn't make sense to keep them permanently. Humans are adaptable creatures.

[youle31]

In my opinion it makes the devs job harder. If someone gives to us a file made with BF bge with and that there is a bug with upbge, assuming his file is 200 mo and has 100 scripts, we'll have to modify his 100 scripts to make it compatible with upbge before we can tell if this is an issue related to backward compatibility or if this is another issue (most of the bug reports are already due to backward compatibility) e.g: https://github.com/UPBGE/blender/issues/275

[organicpencil]

Didn't think about that. Darn. In a perfect world those same changes would be pushed upstream, but I haven't been following the bf/upbge relationship.

[panzergame]

What about generate a script doing the conversion ? like a bash script replacing the non-camel case name. This script could generated after scanning the UPBGE and BFBGE API and looking for attributes removed or simply renamed.

[organicpencil]

Conversion script seems reasonable

[youle31]

Yeah... But as There are not Much underscore properties, as i experienced several cases where removing backward Compatibility was annoying for the dev (e.g: Getmaterial index in a try/except: Pass statement (relief mapping from martinsh. No error in the console), as the user is not always aware of what changes we do, as most of annoying things for users and devs come from backward Compatibility problems, as it generates bug reports, as it costs nothing to add an alias, as i don't care if the code contains 30 more aliases because it doesn't make the code less easy to read, and That it doesn't make the code less performant, as i consider it doesn't make the code awefull or dirt or not clean (too Much perfectionnism sometimes makes our life harder)... For all That i prefer alias solution

[sdfgeoff]

Getmaterial index in a try/except:

Pylint should pull you up if you have a general exception case. I'd argue that that is poor code design from the users point of view. A converter script will catch all this as well (hopefully)

Coming up soon is the 2.8 series, which will (probably) break backwards compatibility anyway, so why not just add this to the things that break when that major release happens? Why not alias? It makes the documentation confusing. For a person developing, they look and see two that do the same and have to figure out which is deprecated. If you could remove the deprecated ones from the docs... then I wouldn't mind.

[youle31]

yes, I thought to remove deprecated from the API

[joelgomes1994]

@youle31 @panzergame @DCubix Hey guys, any news about this? I'm not a C++ coder, but since @youle31 said what to do in the code here in this example:

KX_PYATTRIBUTE_RW_FUNCTION("active_camera", KX_Scene, pyattr_get_active_camera, pyattr_set_active_camera), KX_PYATTRIBUTE_RW_FUNCTION("activeCamera", KX_Scene, pyattr_get_active_camera, pyattr_set_active_camera),

I thought that, as I'm plenty of free time, I can write down the whole API. I just digged the code in the gameengine folder, and it's pretty straight forward to do, and now I can compile UPBGE in my laptop to test the results as well. Actually I already did this editing in some files, and no problems at the compilation until now.

Can I continue to do this, and later, pull request it? If so, how the format will be: thisWay, this_way, or both (to keep the compatibility)? The whole Blender API is this_way, but most of BGE is thisWay... Hard decision. :confused:

I hope I can do, I really want to help. Thanks! :smiley:

[youle31]

Hello, thanks for contribution proposal. Personally, I'm ok to do aliases for attributes like active_camera (have both active_camera and activeCamera). We use camelCase normally in bge API.

What you can do is to: 1) update your upbge sources with : git pull 2) create a branch : git branch ge_aliases for example 3) go on you new branch : git checkout ge_aliases 4) Modify the sources in your ide (Both create aliases in the code and complete the documentation : for example, the doc for active_camera is in E:\Compilation\Blender\upbgeSourcesOld\doc\python_api\rst\bge_types\bge.types.KX_Scene.rst . You can take example on existing doc to add new doc (take care about spaces in documentation and TAB in the code). 5) save your work doing: git commit -a -- This will open a text editor (generally VIM). You have to enter a commit message. To do so, press i to enter in edit mode (i like insert), then write a commit message like add alias for active_camera, then quit edit mode doing escape, then save the commit and quit VIM writing :wq (like :write and quit) and validate with enter.

Your work is now in you branch ge_aliases. You can do several commits modifying the sources then doing again git commit -a...

If you want to create a patch, you can do: git diff master ge_aliases > aliases.diff It will create a file named aliases.diff in your upbge sources folder. We can apply this patch on upbge sources after review.

But few things:

• A big part of the API will disappear with eevee
• The patch has to be approved by panzergame

@panzergame: For me, I'd be ok to create aliases. What do you think? Is it the good moment to begin such work? and are you ok with aliases?

[joelgomes1994]

I tried some test editing and generated the Python API... Would it be better this way:

Or this way:

For now, I assume the second way is the best, since we can have a direct link for each attribute.

I'm adding some missing attributes in the docs as well (example, layer and layerWeight in BL_ActionActuator).

[youle31]

For me the first solution is good

[joelgomes1994]

It's more readable, but the formatting in RST doesn't help... See how the function arguments are bold:

In the other side, when using two lines, the formatting is maintained:

The RST is written as '.. method:: getButtonStatus(button_index)'. I don't know if this can be manually fixed, but make a fix for all the functions and methods will be hard. What's the solution, keep it in a single line and bold, keep in two lines with the correct formatting or other solution?

[youle31]

@joelgomes1994 :

• wait, when I say that I'm ok to add aliases, it's only for attributes which are NOT ALREADY in camelCase. The norm in bge API is camelCase.
• about the format, better to wait panzergame opinion.
• iirc, he talked about a way to automatically accept both camlCase attribs and not camelCase attribs. Better to wait for his opinion on the subject.

Anyway, you can still train but I can't guarantee your work will be accepted. It's panzergame who decides

[joelgomes1994]

Oh, ok then. I will revert the wrong editings.

[panzergame]

My opinion is maybe extreme, but I think that words that new users should not use should be not present in documentation, else we only suppose that the user is kind and prefer the typo we try to impose but it's rarely the case.

So I think that the non camelCase words should be not present in documentation but only kept for compatibility purpose.

[youle31]

I agree with panzergame. I propose this as template for future modifications: http://pasteall.org/636278/diff

[joelgomes1994]

Ok then, I'm working on it. I'm updating the docs as well, adding missing definitions, fixing wrong punctuations, etc... Should I remove the deprecated content from the docs as well (like KX_GameObject's scaling, location, etc) as the user should not be using these as well?

[panzergame]

@joelgomes1994 : I think, just add these modifications in a separate branch or commit.

[joelgomes1994]

@panzergame It's already in my local branch called 'ge_aliases', I will push the branch and upload the diff for revision just after I finish the job.

[joelgomes1994]

Current status of editing. I've get the spreadsheed from @sdfgeoff 's issue #532 and updated it. I've marked all the new aliases names added to the source code as 'Complete', and keep as 'In Progress' and marked yellow everything which is deprecated, must be refactored, etc. Some of the aliases are marked as 'Complete' and marked as yellow, which means that the new alias has been added, but the attribute may be changed in future (like KX_Camera's shiftX and shiftY become a vec2 shift). In the end, everything marked as yellow is something I can't (or don't know how to) edit. Now, I will just finish the docs and upload the branch / diff. https://docs.google.com/spreadsheets/d/1r82Mx1OFGl0G4G0M_U8__YlPztpMOP-9E3l-g3shdfQ/edit?usp=sharing

[joelgomes1994]

@panzergame , here it is the GIT patches, the full edit list is in the commits descriptions. Python API Aliases GIT Patches.zip

Spreadsheet updated: https://docs.google.com/spreadsheets/d/1MzQU2N4YV8owvnZ6TUV6EzvvaY-g7rWd6cLXXeoxbQY/edit?usp=sharing

[youle31]

Very good job! Thanks very much for this. After a first read, it looks good to me (I was a bit hesitant about ImageRender/KX_WorldInfo background doc removal and also joysticks attribs which are deprecated, but this is time to remove it I guess)

[joelgomes1994]

I'm glad you liked, @youle31 ! :smile: I think the API reference gets a lot more readable and consistent after removing deprecated content, especially the redundant ones (logic.setGravity, constraints.setGravity, scene.gravity, for example), and avoids people using it for new projects... And if they try, the deprecation warning is printed at the console to remember they shoudn't be using it. After digging through the API reference RST files, I was planning to fill up the reference with some more stuff (like code examples, detailed descriptions, etc, something like TutorialsForBlender3D). But let's see after the review of the patches, and if you guys give me permission the write this up. :wink: