-
Notifications
You must be signed in to change notification settings - Fork 200
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
std::function return value for OryolClassCreator #320
Conversation
Far from perfect, but it should increase support for using arguments in the creator.
Already tested, and it works both with and w/o the workaround, and can still be implicitly cast to std::function. Only potential downside is if someone's been using `auto f = Type::Creator()`.
I like the GNU version fix, but not the std::function fix :) Can you give a code example where wrapping in a std::function is needed? I think a second set of macros would make sense. Basically leave the current OryolClassCreator() as it is, and add another macro OryolClassCreatorStd(), so I can decide case by case whether I want the function pointer or std::function (and this would also not break existing code) |
Uh, it's the other way around. It wraps in a std::function now, my change gets rid of that and uses raw function pointers. |
Unfortunately, the args capture in template<typename... ARGS> static Ptr<TYPE>(*Creator(ARGS... args))() {\
return [args...] { return Create(args...); };\
}; breaks the lambda because the type of the lambda is incompatible with the return type. |
...oops? |
Yeah sorry for getting the function-ptr / std::function thing mixed up, guess I didn't have my morning coffee yet... |
PS: we shouldn't consider std::bind, I still feel the scars (I guess that's why I was switching to std::function and, the easier argument capturing was the reason)... I think it's best if there's an alternative Creator macro which which returns a function pointer but doesn't support args capturing. |
Alright. Another important note is that this appears to not work on Emscripten, so it might be best to leave it as std::function and not add a new macro. |
Hold on, I take that back. There is a problem occurring on Emscripten, but I'm now pretty sure this wasn't causing it. |
Yeah, this wasn't the problem. #322 |
On the other hand, this does break the creator for LocalFS on Windows, and I don't even remember why i wanted it. Going to close this. If I come up with a better solution in the future, I'll let you know |
Far from perfect, but it should increase support for using arguments in the creator.
I tried using e.g. std::bind as a better workaround as well, without success.