Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vlad0337187 opened this issue Nov 9, 2018 · 6 comments
Open

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

vlad0337187 opened this issue Nov 9, 2018 · 6 comments

Comments

@vlad0337187
Copy link

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
Copy link
Collaborator

paul-marechal commented Nov 9, 2018

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
Copy link
Collaborator

paul-marechal commented Nov 9, 2018

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
Copy link
Author

vlad0337187 commented Nov 10, 2018

@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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants