-
-
Notifications
You must be signed in to change notification settings - Fork 402
Java function rework #7969
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
base: dev/feature
Are you sure you want to change the base?
Java function rework #7969
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a huge improvement over the old functions, good job!!
src/main/java/ch/njol/skript/lang/function/DefaultFunction.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/lang/function/DefaultFunction.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/lang/function/FunctionArguments.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/lang/function/DefaultFunction.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/lang/function/DefaultFunction.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/lang/function/FunctionArguments.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/DefaultFunctions.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/DefaultFunctions.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/DefaultFunctions.java
Outdated
Show resolved
Hide resolved
Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>
…to feature/java-function-rework
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I like the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just marking that I need to review this again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks pretty good. almost ready
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
Problem
In preparation for named function arguments, which allows changing the order of appearance of function arguments, a replacement has to be created for JavaFunctions. JavaFunctions use indices to get function arguments, which is incompatible with the planned design for functions.
Other than that, registering functions is just really ugly. This PR improves that.
Old

New

This is a part of the larger function rework.
Solution
A DefaultFunction has several advantages over a JavaFunction.
String[].class
.location
function, the amount of worlds would sometimes be more than one. This is bad design, as theworld
parameter is defined as being single. DefaultFunction now correctly selects a single world.since
method.Testing Completed
Supporting Information
Completes: none
Related: #7590 (since array support)