To be honest I don't know if it is seriously possible but I think it would be a nice to thing to also be able to integrate it with Skillable ^^
you could do some requirements for e.g. crafting a sword but you need a hunting lvl of X because it is pretty strong etc.

I don't know how many people would have a use of this but I think it wouldn't hurt^^
like already said take your time this is just a Suggestion what you could add :P

If Skillable had an API, implementing a feature like this would be easier. I rooted around in the source for a few minutes and it looks possible, just not all that obvious or guaranteed to be forward compatible.

I might take a look at this again at some point.

Worth noting there is a fork called Reskillable in the works which will probably be better able to be supported

Thanks @SkySom, it looks like everything that I'd need is there.

One thing that might be handy for users is a command to dump the names of all skills to the console or a log file.

My builder method will probably look something like this:

// addReskillableRestriction(String skillName, int minLevel, @Optional int maxLevel);
builder.addReskillableRestriction("reskillable.mining", 10);

I think it would facilitate usage if users could get at and display the names of all registered skills.

Do you know when Reskillable will be released?

Just popping my head in to comment this (Since Reskillable was mentioned) I just rewrote several portions of ReSkillable such that there is an API for Custom Traits/Skills and being able to check that. If there's any thing specific we could do to assist you, give me a heads up!

That's not a bad bit of functionality to add! Will add it to the todo list. The hope is some time this week. Also I may suggest doing a format matching our RequirementHolder! (AKA reskillable:mining|10) As it'd allow for any of the Requirement locks we allow! (Any number of Skill/Trait/Advancement locks with us working to open up the api to allow for more types) One thing to note using Registry names may be best. The version with the period is mostly used for localization.

Very nice! I didn't really see the RequirementHolder stuff before; it's obviously the way to go. Thanks for pointing that out to me.

I've done some refactoring to allow mods to add their own recipe requirements, but, more importantly, it will allow me to build out recipe requirement integrations without having to bloat the recipe builder with dependency specific methods.

My revised builder methods will probably look something like this:

builder.addRequirement(Reskillable.requirement().add("reskillable:mining|10").add("reskillable:frolicking|42"))

Will there be documentation on the string format for the RequirementHolders? That's not something that I really want to maintain and if I could just provide a link to Reskillable's documentation that would be ideal.

Also, does Reskillable have a maven?

There will be. The config itself in Reskillable does show the current format, but we're working out how best to document it and where (Especially since there's the ability to add custom Requirements. May add a Github wiki page. Will poke Lanse. And maven is actually on my todo list for today. Will update when I get it added, but it'll be on https://maven.blamejared.com

And maven should be working now: https://maven.blamejared.com/codersafterdark/reskillable/Reskillable/1.12.2-1.0.0-SNAPSHOT.3/

Ok, @SkySom, I've implemented the integration, but it's not working as expected.

The requirement match works as expected on the server, but not the client. PlayerData#matchStats always returns true on the client because the player object is not an instance of EntityPlayerMP.

That actually explains some of the bugs, that I've been trying to scope out, and been unable to figure out. Now I feel rather silly. Thank you for the heads up on that!

I've implemented a temporary, hacky solution to this problem to test that the integration works, and it does. I'm going to mark this issue as done, but leave it open to remind me to cleanup this code when Reskillable gets the issue sorted on their end.

Should be fixed in the release version (I hope) We'll make sure to double check it.

Confirmed working with SNAPSHOT.13

Confirmed working with 1.12.2-1.0.0

Thanks for all your help @SkySom!

No thank you! I'm happy to see all the work I did on improving the api actually be useful!