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

Binding a free function as a non-static member function #20

Open
OverShifted opened this issue Jan 3, 2023 · 5 comments
Open

Binding a free function as a non-static member function #20

OverShifted opened this issue Jan 3, 2023 · 5 comments

Comments

@OverShifted
Copy link

OverShifted commented Jan 3, 2023

I'm trying to bind a free function, as a non-static member function of a foreign class.
Here is my Wren code:

foreign class MyData {
	foreign value
	foreign value=(rhs)

	foreign twiceValue

	toString { "MyData(%(value))" }
}

// Some where else
class Player {
	// ...
	static puts(obj) {
		System.print("Putting: %(obj)")
		System.print("Putting twice: %(obj.twiceValue)")
	}
}

And here is my C++ code (module names are handled correctly):

class MyData
{
public:
	double value;
};

double twiceValue(MyData& self)
{
	return self.value * 2;
}

// *m_VM is a thin wrapper around `wrenpp::WrenVM` which provides `beginModule` as a wrapper around wrenpp`s version of it.
m_VM->beginModule("src/wren/lib/components")
	.bindClass<MyData>("MyData")
		.bindGetter<decltype(MyData::value), &MyData::value>("value")
		.bindSetter<decltype(MyData::value), &MyData::value>("value=(_)")
		.bindMethod<decltype(twiceValue), twiceValue>(false, "twiceValue")
	.endClass()
.endModule();

// Loading modules...

m_VM->GetWrenpp().method("src/wren/test", "Player", "puts(_)")(MyData{12});

When I run it, I get:

Putting: MyData(12)

And then it segfaults.
Using a static function (binding twiceValue as static and passing instance as its first parameter) or a member function works fine.

@ConorDamery
Copy link

ConorDamery commented Apr 29, 2023

I'm not a contributor to this repository, but I'm fairly familiar with the codebase. Anyhow, here's my 2 cents.

Firstly, why do you have a free function that takes a reference to MyData? Don't you think it'd be easier to simply put it inside the class? Also, since you are placing it outside of the class, it really should be static.

Secondly, you are binding to a function that expects one parameter, but you provide none in the Wren version, so this won't work.

Here's my advice, put the method inside the class and you can remove the self-reference parameter, then the code would do what you expect it to and it will be cleaner too, hope this helps!

@OverShifted
Copy link
Author

@ConorDamery Thanks for the reply!
I'm trying to keep my C++ API simple and avoid adding extra methods. Sometimes I can't even do that because of some "C-style" APIs. For example, I'm using a library that provides a vec3 class (Its C++), along with a length free function which computes the length of the vector. The vec3 class is bound to wren along with a constructor. I wish I had a .length property on the wren class which calls the C++ length function under the hood. In this situation, I can't add a new method to the C++ class.

@ConorDamery
Copy link

ConorDamery commented May 1, 2023

You can still do this. I'm going to assume the math library you are referring to is glm?
Anyway, there's nothing preventing you from wrapping glm into your own vec3 class which mirrors the wren implementation.
And if you really don't want to do that, then you can still mirror the wren implementation by providing a static method in your wren class which takes a const vec3 reference.

For example:

// wren
foreign class vec3 {
    // constructor, etc...
    foreign static length(v)
}
// cpp
.bindMethod<decltype(glm::length<glm::vec3>), &glm::length<glm::vec3>>(true, "length(_)")

@OverShifted
Copy link
Author

OverShifted commented May 2, 2023 via email

@ConorDamery
Copy link

ConorDamery commented May 3, 2023

Alright fair enough, well my only other approach is to try hacking it using wren semi-directly, which should work in theory (the code is not tested!).

.bindCFunction(false, "length", [](WrenVM* vm)
{
    const glm::vec3& v = WrenSlotAPI<const glm::vec3&>::get(vm, 0); // First get the object from slot 0
    float l = glm::length(v);
    WrenSlotAPI<float>::set(vm, 0, l); // Now we set the return value to slot 0 as usual
})

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

No branches or pull requests

2 participants