FunctionManager: implement framework for handling "info" functions #291
Conversation
common/src/main/java/com/wynntils/core/functions/FunctionRegistry.java
Outdated
Show resolved
Hide resolved
magicus
changed the title
|
The template parser is still missing. It's a bit more complex to get right, so I'd like to return to it in a separate PR. Otherwise I'm pretty happy with this design. The two test functions should be removed when we start to get some real functions in their place. |
magicus
requested review from
kristofbolyai,
DonkeyBlaster,
HighCrit and
P0keDev
…ence to the function itself)
common/src/main/java/com/wynntils/commands/wynntils/WynntilsFunctionCommand.java
Outdated
Show resolved
Hide resolved
|
This can now use new managers |
|
@kristofbolyai I'm not sure how new managers would fit in. I think I'll keep the function init as a separate step, with I18n, close to the feature init (after the latest i18n init fix). Am I missing something? |
common/src/main/java/com/wynntils/core/functions/FunctionManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/core/functions/FunctionManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/core/functions/FunctionManager.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/core/functions/EnableableFunction.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/core/functions/EnableableFunction.java
Outdated
Show resolved
Hide resolved
| public class TestFunction extends Function<Integer> { | ||
| @Override | ||
| public Integer getValue(String argument) { | ||
| return 42; |
There was a problem hiding this comment.
There is no need for this function to exist. We have a real world example.
There was a problem hiding this comment.
I'll remove it as soon as we have real functions. But I'm not sure what real world example you are thinking of..? I do need some kind of test function in this PR, so I'm planning on removing this in a follow-up when we start to port over legacy functions.
There was a problem hiding this comment.
And with a real world example I am referring to an actual implementation as we might expect in production
There was a problem hiding this comment.
Ok, I'll port over a handful of "info variables" from legacy, so we can see this API in action. It's a fair point, which I'm usually trying to make. ;-)
There was a problem hiding this comment.
We lack the source data for most of our info variables. I made a list at #301 for keeping track of what is still left to do. I implemented two categories of functions: those that derive their value from Minecraft, and those that derive their value from the general environment (basically, the JVM).
There was a problem hiding this comment.
Ok, I think I got this covered for now. If anything, we should perhaps try to get some ActiveFunctions as well. I'll see if there is any I can implement without far too much work.
There was a problem hiding this comment.
For some reason my comment disappeared regarding the already present worldnamefunction :?
…update time * Fix raw access template warnings
No description provided.