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

Rework function system #2383

Merged
merged 11 commits into from
Sep 1, 2019
Merged

Rework function system #2383

merged 11 commits into from
Sep 1, 2019

Conversation

bensku
Copy link
Member

@bensku bensku commented Aug 18, 2019

Description

This pull request contains a rework of Skript's function system.

  • Reduced code duplication between Function and Signature
  • Reworked function loading and unloading to work with namespaces
    • No visible changes to scripts
  • Cleaned up and commented code

More testing will be needed before this can be merged.


Target Minecraft Versions: any
Requirements: none
Related Issues: #2337, #2378 and potentially other function-related issues

@bensku bensku requested a review from a team August 18, 2019 15:39
@bensku bensku changed the title Feature/better functions Rework function system Aug 18, 2019
@andrei923
Copy link

What about #2188

@Matocolotoe
Copy link
Contributor

Maybe you could change the error message to "none is given" instead of "but 0 are given" when the function doesn't have enough arguments.

@noftaly
Copy link
Contributor

noftaly commented Aug 26, 2019

What about function overloading ? #1185 It would have been nice to add this
And what about #2353 ?

@Matocolotoe
Copy link
Contributor

What about #2345 ?

@bensku
Copy link
Member Author

bensku commented Aug 31, 2019

#2188 is out of scope, but foundations for implementing it are done. Same goes for #1185, if we end up implementing it. I'll look at other issues.

@bensku bensku mentioned this pull request Aug 31, 2019
@bensku bensku merged commit f1960e5 into master Sep 1, 2019
@Whimsyturtle Whimsyturtle deleted the feature/better-functions branch July 25, 2020 00:50
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

Successfully merging this pull request may close these issues.

4 participants