This patch adds a new method getActionNames() to the KX_Scene Python class.
Goal is to be able to check what actions currently exist in the scene, it is particularly useful when working with LibLoad.
We could also do it for objects, but then references in ActionManager only take in account currently playing actions.
The naming of the function follows KX_GameObject.getPropertyNames(), but could be changed if needed.
The patch also adds doc as needed.
Details
Diff Detail
- Repository
- rB Blender
- Branch
- arcpatch-D1411
Event Timeline
| doc/python_api/rst/bge_types/bge.types.KX_Scene.rst | ||
|---|---|---|
| 189 | This is just rewording the method name, which doesn't add any information. Your description of the patch contains much more info, why not include that information in the documentation? You may also want to include a reference to this new method to the documentation of KX_GameObject.playAction; I was thinking something along the lines of "See <reference here> to obtain a list of the action names that are currently loaded and available." | |
| 191 | You could describe it as "list of strings" or "list of str objects", so that it's explicit what's in there. | |
| source/gameengine/Ketsji/KX_Scene.cpp | ||
| 2594 | Is a list really the best data structure for this? If the order doesn't matter (and I feel that this is the case), then a set would be better. With a set a query like actionname in scene.getActionNames() will be more efficient. | |
| source/gameengine/Ketsji/KX_Scene.cpp | ||
|---|---|---|
| 2594 | Campbell prefer tuple (PyTuple_SET_ITEM) because the are not mutable. Code not tested, but should work. PyObject *list = PyTuple_New(act_map.size());
for (int index = 0; index < act_map.size(); ++index) {
PyTuple_SET_ITEM(list, index, PyUnicode_FromString(*act_map.getKey(index)));
} | |
- Change list to tuple, add doc.
| doc/python_api/rst/bge_types/bge.types.KX_Scene.rst | ||
|---|---|---|
| 189 | Ok, I add a bit of info. I also include references to KX_GameObject.playAction, BL_ActionActuator.action and BL_ShapeActionActuator.action. | |
| 191 | Updated to "tuple of strings". | |
| source/gameengine/Ketsji/KX_Scene.cpp | ||
| 2594 | I also thought that it would be more logical tu use tuple, but I saw that KX_GameObject.getPropertyNames uses list. I change to tuple for now. Seems like PyTuple_SetItem is safe in our case (topic on stackoverflow), which one is prefered? Note: Sets could be good too, and frozensets are available if immutability is needed. What do you think, @Sybren A. Stüvel (sybren), @Thomas Szepe (hg1) and @Campbell Barton (campbellbarton)? | |
Can you also upload a Blend file that demonstrates this new feature? You'll also need that for the release log wiki page anyway.
| source/gameengine/Ketsji/KX_Scene.cpp | ||
|---|---|---|
| 2601 | From the stackoverflow post you linked, it shows that you can use PyTuple_SETITEM (note the different capitalisation). It's slightly faster because it doesn't do any error checking, which is fine as the tuple is brand new and we're 100% sure about that. | |
- Change PyTuple_SetItem to PyTuple_SET_ITEM, should be a bit faster.
For the .blend file: I could make a simple one, but as previously said, the feature is particularly useful for LibLoad.
Are .zip files containing multiple .blend allowed for the release notes?
Good question. I think it depends on how much functionality of SCA_LogicManager is worth exposing to Python as well.
For the .blend file: I could make a simple one, but as previously said, the feature is particularly useful for LibLoad.
Actually for me it is not clear why this patch is needed. Because if you want to plan an action by using an action actuator or the playAction() methods, you need to define the action names. So please provide an example file which is showing the use case.
Are .zip files containing multiple .blend allowed for the release notes?
Yes, why not?
The .rst documentation changes in BL_ActionActuator.rst, BL_ShapeActionActuator.rst, KX_GameObject.rst are not necessary and should be removed.
Basically I think the documentation should only describe what the method or attribute is doing and not what other method's are exiting. Except for deprecated methods or attributes.
| source/gameengine/Ketsji/KX_Scene.cpp | ||
|---|---|---|
| 2595 | One space after the data type. | |
@Quentin Pointillart (Quentin) Wenger (Matpi). I accidentally selected commandeered revision. So the author has changed. I don't found a way to change it back. So please check if you can commandeered revision back to you.
Thanks for the review, I'll make changes as soon as I can (got no time now, exam weeks at the university).
I disagree with this. When several methods and attributes are related, this relation should be documented as wel. It makes documentation more easy to read, and to be honest I don't see a downside.
I don't agree with your point of view.
- I think the API documentation should only describe the API it self, like the rest of the blender documentation.
- Also it will not be uniform to the rest of the documentation.
- If we do this here, we will end up in documenting every function which is related to an other function. Like KX_GameObject.name and KX_Scene.objects or KX_Scene.objectsInactive. Same for lights, cameras, active_camera and so on. Which makes the documentation harder to read (longer documentation more to red more to understand).
- Non programmers should use LogicBriks or finished scripts. A programmer should know what he does and how to use the search function in the documentation.
- When I use a attribute like "action" on the actuator, I know that I only get this action name. So pointing me to the KX_Scene.getActionNames() is useless. Because when I want to get the currently used action name for the actuator, why I should be interested in getting all actions of the whole scene.
- Also for me, getting the the actual setted action name is not really related with a list of all available action names for this scene.
Basically I think when we get starting here doing something like this, we will end up in an over documentation mess with a lot of useless information and links.
I think that the rest of the Blender documentation is also pretty terse, and really could use some extending.
It's an ongoing effort. Of course we can overhaul the entire documentation, but nobody's got time to do that.
I agree that we should find a nice balance between brevity and clarity. However, at a point in a lot of documentation where they say "here you input any x ϵ X", I ask myself "ok, where do I find X?" -- in my opinion, this should be part of the documentation. A programmer will not instantly know all of the API by heart. Having cross-links in the documentation can help tremendously in understanding how things fit together.
When I use a attribute like "action" on the actuator, I know that I only get this action name. So pointing me to the KX_Scene.getActionNames() is useless. Because when I want to get the currently used action name for the actuator, why I should be interested in getting all actions of the whole scene.
Don't forget that it's also a setter. Of course for just a getter this link would be utterly irrelevant, I agree, but when you need to set something by picking a string from a given set of strings, pointing to the location where you can find that given set of valid values seems like a good idea to me.