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

Squirrel: Default argument support for exposed C++ functions #3025

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Vankata453
Copy link
Member

This PR updates the simplesquirrel submodule to a branch which supports exposing C++ functions (including class constructors) wtih default arguments. It's not yet been merged into simplesquirrel master, because the Squirrel submodule commit branch used is also not master to allow for an important change to Squirrel parameter checking to be included.

Only the Camera::scale() and scripting::Level::spawn() functions were modified to support default arguments and their respective other functions were marked as deprecated. In case more functions are found suitable for this, this PR would be updated.

Copy link
Contributor

@Brockengespenst Brockengespenst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested Level.spawn() with all transitions and everything works as expected.
I did not test the camera stuff.

This PR updates `simplesquirrel` to a branch which supports exposing C++ functions (including class constructors) wtih default arguments. It's not yet been merged into simplesquirrel `master`, because the Squirrel branch used is also not `master`, so an important change to Squirrel parameter checking is included to allow for default argument support.

Only the `Camera::scale()` and `scripting::Level::spawn()` functions were modified to support default arguments and their respective other functions were marked as deprecated. In case more functions are found suitable for this, this PR would be updated.
Copy link
Member

@Frostwithasideofsalt Frostwithasideofsalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works.

Copy link
Contributor

@tylerandari13 tylerandari13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Patrick Star once said, "It can't be that bad. Looks good to me."

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

Successfully merging this pull request may close these issues.

4 participants