Skip to content

Add advancements#760

Draft
OhmV-IR wants to merge 4 commits into
masterfrom
add-advancements
Draft

Add advancements#760
OhmV-IR wants to merge 4 commits into
masterfrom
add-advancements

Conversation

@OhmV-IR
Copy link
Copy Markdown
Contributor

@OhmV-IR OhmV-IR commented May 19, 2026

Tasklist:

  • Get criterions working properly
  • Successfully get a root advancement working (new page)
  • Basic config design & serializer
  • Split child and root advancement node classes (justin request)
  • Custom criteria

I have a very high degree of confidence in this code getting roasted in CR

RebarRegistry.ADVANCEMENTS.register(this)
}

fun vanilla(): Advancement = Bukkit.getAdvancement(key)!!
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo should be nullable, I don't think this should throw an exception if called before register()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the config adapter calls register and I think I'd just end up null asserting everywhere else so not gonna bother

Comment on lines +116 to +130
val resourceKey = identifierFromKey(key)
val mapBuilder = ImmutableMap.builder<Identifier, AdvancementHolder>()
mapBuilder.putAll(MinecraftServer.getServer().advancements.advancements)
val advancementHolder = AdvancementHolder(resourceKey, nmsAdvancement)
mapBuilder.put(resourceKey, advancementHolder)
MinecraftServer.getServer().advancements.advancements = mapBuilder.build()
val tree = MinecraftServer.getServer().advancements.tree()
tree.addAll(listOf(advancementHolder))
val node: AdvancementNode? = tree.get(resourceKey)
if (node != null) {
val root = node.root()
if (root.holder().value().display().isPresent) {
TreeNodePosition.run(root)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any way at all to make this more readable

Copy link
Copy Markdown
Contributor

@JustAHuman-xD JustAHuman-xD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to make these comments like a week ago or something and never finished the review lol

)
val itemIsVanilla = RebarItem.isRebarItem(item)
item.amount = criteria.itemCount
for (player in Bukkit.getOnlinePlayers()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't check if the player has the advancement or pre-reqs already


object Ticker : Runnable {
// I don't even want to think about the performance of this method...
override fun run() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method should be restructured a bit too, first get all of the advancements that have this criteria, if none, return early, then the player loop should be the top level & it should iterate over the inventory only once and gather all of the types (using the wrapper) -> counts, then iterate over the advancements, if they do not have the advancement and have its parents/pre-reqs, then check if they meet it

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.

2 participants