UPBGE / upbge

UPBGE, the best integrated game engine in Blender
https://upbge.org
Other
1.46k stars 182 forks source link

function takes 2 args, should be zero or 1 controller arg #929

Open vlad0337187 opened 6 years ago

vlad0337187 commented 6 years ago

Hello.

Met such issue when tried to launch function scripts.main.helpers.characters.set_player_camera_active from Python controller: screenshot from 2018-11-09 08-39-30

It's comes from this code: screenshot from 2018-11-09 08-42-12

It checks amount of arguments, but it doesn't account that this arguments can have default values to be able to launch this function and from controller, and from other code.

Suggest to get amount of default values before, than check: how many arguments it requires. Wrote this in Python-pseudocode as I don't really know CPP (len - function, which returns length of passed to it list or tuple (PyObject)). screenshot from 2018-11-09 08-48-50

So if all defaults provided - function would run normally. If one arg without default value - that there would be passed controller later. If more args without default values - error.

paul-marechal commented 6 years ago

The GE code also fails at inspecting bound method function-likes, as well as custom-made callables in general (instances which class defines a __call__ method).

The workaround is to make a wrapper specifically for the GE to inspect:

def some_function(controller, b=1, c=2):
    ...
some_function_controller = lambda controller: some_function(controller)

# or with a regular function:
def some_function_controller(controller):
    return some_function(controller)

My concern is that it is already a pain to inspect the function from the C++ code. If we can keep it simple and simply provide adapted functions that it will more easily inspected would be the best IMO.

paul-marechal commented 6 years ago

Moreover, in your code you have parameters, but none of them seems to correspond to a controller, which the BGE is trying to pass. So how would the GE understand what parameter should be a controller or not?

Since we are using Python 3.7 now, we could use annotations:

def some_function(a: 'controller', b=1):
    ...

But then again, if the function wrapping works for you, I think that it would be better to just do that.

In your case, it would look something like the following:

def set_player_camera_active(player_cam_name=None, scene=None):
    ...
set_player_camera_active_controller = lambda: set_player_camera_active()
vlad0337187 commented 6 years ago

@marechal-p ,

The GE code also fails at inspecting bound method function-likes, as well as custom-made callables in general (instances which class defines a call method).

Strange, I often use callable classes instead of regular functions. For example, I made so recently on 0.2.4:

from scripts import jiggle_bone

update_boobs = jiggle_bone.BreastMotionLight (
    left_breast_name    = 'boob_r',
    right_breast_name   = 'boob_l',
    rig_name            = 'mira_armature',
    horizontal_axis = 'z',
)

jiggle bone module: https://pastebin.com/5YFFLsKt

I agree with you that code should stay clean. But is that check needed at all ?

I suggested approach only to check: (total_args - args_with_defaults) instead of total args. Other stay unchanged: if there's one arg - controller is passed to it, if no - nothing is passed.

args = PyTuple_New(1);
PyTuple_SET_ITEM(args, 0, GetProxy());
resultobj = PyObject_CallObject(m_function, args);

Of course, there are workarounds, but that seemed to me a one-two lines fix, so I suggested it =) But it's you who can decide: is it needed or not )

paul-marechal commented 6 years ago

I suggested approach only to check: (total_args - args_with_defaults) instead of total args.

Sounds simple enough :)

Strange, I often use callable classes instead of regular functions.

I looked at your example, __call__ did not take any parameter. If you define __call__(controller) the BGE will not inject the controller when calling it, but it will indeed call it. Then Python throws of course.

vlad0337187 commented 6 years ago

I got it, thanks.

What I thought, is it needed at all to pass controller to Python callable ? Is it faster than to call bge.logic.getCurrentController () ?

paul-marechal commented 6 years ago

What I thought, is it needed at all to pass controller to Python callable ? Is it faster than to call bge.logic.getCurrentController() ?

No idea, but I like the idea that your dependency is passed from the caller, rather than asking for a context info. bge.logic.getCurrentController is also callable from places where it shouldn't (inside pre_draw event), but if it is passed to you only when it can be, great good in my opinion.