[WIP] Advancement API

[Machine-Maker]

Creating this for myself. Various additions relating to advancements

• Advancement manager to edit/create advancements
• Expand the Advancement interface
• added RequirementsBuilder for building requirements 2d arrays
• added interfaces for all possible criteria
• add implementations for above interfaces
• add validation checks when constructing a criteria
• add AdvancementDisplay class
• add AdvancementRewards class
• update various advancement events
• persistence through /minecraft:reload
• find more bugs

Will add more as I think of/remember stuff.

[Machine-Maker]

So looking more into this, it should also be possible to add a config option for per-world-advancements. Might be useful for some

[VADemon]

Not strictly per-world, but worlds that share the same "pool" of advancements. Example:

Pool1: world + world_mining + world_the_end
Pool2: world_nether + world_pvp

[Machine-Maker]

Yeah, I guess that would be the best way to do it. And that should probably be setup in the paper.yml, rather than programmatically. Since it should only be set once.

[Trigary]

Resources that might help developers:

• an implementation that contained everything at one point, but has many design flaws and is now probably outdated: https://github.com/Trigary/AdvancementCreator
• great documentation that implementation is based on, but seems to be outdated: https://github.com/skylinerw/guides/blob/master/java/advancements.md

[Machine-Maker]

@Trigary so I see you just created the JSON file and then called reloadData to load in the custom advancements.

Would it be better to implement it like this, or better to hook directly into mc's AdvancementDataWorld class and add advancements purely programmatically?

[Proximyst]

For a server API, using workarounds is not the greatest of ideas; if you can do it programmatically, prefer that.

[Machine-Maker]

So as I continue to figure out exactly how the code is written for advancements, I think its probably best to hook into the existing JSON parsing that the advancements do. Especially the trigger stuff. Using as much of the json parsing also limits interaction and, more importantly changes to NMS code that break way more easily.

[Machine-Maker]

So it should be almost done. I still have to go through and add implementations for all the various triggers, but everything else should be there.

[Machine-Maker]

Ok, should be only bug fixing left to do. I'd love some feedback on this whole thing, I'm sure I'm not using best practices in a bunch of places.

[ghost]

excited for this to be merged.

[Machine-Maker]

@dudeman123 its not 100% done yet. There is still a bunch of testing that has to be done. and all the javadocs for it.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Machine-Maker]

Still plan on working on this in the future

[ghost]

When will this get added?

[electronicboy]

Please don't use the issue tracker to ask for ETAs, this PR is not even finished yet.

[Machine-Maker]

So I'm trying to decide how to "setup" all the objects here, cause right now, (after I push latest changes) this PR will add at least 133 classes, most of which have to do with advancement conditions, triggers, predicates, etc. (there are a ton of them). And I'm wondering what the best way to have the API create them. Should they be mutable (getter/setter) or something like this? I don't want to create all these only to have to go back and change it later 😅

[Machine-Maker]

Well I decided to just push what I have so far, before I go further tho, I'd like some feedback on the general structure, or if this should even be a thing in the first place.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kezz]

Commenting to unstale. This could even be implemented completely with adventure as it's a new API 👀

In order to make this an actually useful comment I'll leave some feedback:

• The announceToChat attribute should be removed. Chat announcing is already controlled with a gamerule so it feels odd to let plugins decide this upon creation. Perhaps including an AdvencementAnnounceEvent would be better? Might be odd to work with per-world gamerules for this one.
• Perhaps the same with showToast. Could just be an event.
• For rewards, it feels like this should just be left to events as well. Plugins can easily listen to advancement events and just award rewards. As rewards aren't possible with vanilla advancements, it doesn't seem like the defacto way to add advancement rewards should be like this.
• I presume the triggers will be informative only? As in, plugins won't be able to make triggers for advancements? I think this makes sense as choosing when to award advancements feels best left up to the plugins to implement.
• PotionType changes would be better served with an overloaded constructor that defaults to the lowercase version of the name of the enum. Would save a few diff lines 🤷
• Would it be better for the getByKey methods to use static maps instead?

[Machine-Maker]

@kezz so the annouceToChat and the showToast are really there to mirror the nms advancement. When you create your own advancement via a datapack, you have those options available to you.

* As rewards aren't possible with vanilla advancements

They are possible with vanilla advancements? You can specify various rewards for completing an advancement.

[Machine-Maker]

Closing this until I come back to it. Slowly am working on the structure of it separately here.