mc-datapacks / review-tracker

Datapack Review tracker
6 stars 1 forks source link

Ding Dong #21

Closed al-beqaa closed 3 years ago

al-beqaa commented 3 years ago

Project Page: https://github.com/ChromaKey81/ding-dong

al-beqaa commented 3 years ago

download page: https://www.planetminecraft.com/data-pack/ding-dong-4801059/

al-beqaa commented 3 years ago

Just updated the pack to follow the advancement convention

UltroGhast commented 3 years ago

Sorry, your datapack can't be certified. ❌Compatibility Verification ❌Stamp of Quality

Global Ignoring Tag:

advancement.mcfunction

in unless entity @e you should put tag=!global.ignore in the selector in title @s actionbar you should put tag=!global.ignore.gui in the selector

Datapack Advancement:

You shouldn't put the datapack advancement in the global folder. Citing the Convention Wiki:

You can create this advancement anywhere, as long as you don't pollute /data/global/advancements/folder.

Also, I suggest you to put your custom tags in your data folders and not in the minecraft one

al-beqaa commented 3 years ago

The advancement is in the right spot? It is under global/advancements/chromakey. The file under global/advancements is the root for all datapacks, which every datapack is supposed to have.

I have just updated the code to include global.ignore requirements you pointed out.

(The block tag under minecraft namespace is not custom btw, it is used in vanilla, which is why it needed to be in the minecraft namespace. The game uses it to determine which entities to highlight when a player rings a bell.)

UltroGhast commented 3 years ago

-The root advancement MUST BE in global/advancements. -The creator advancement can be everywhere but with the creator namespace and always in the same directory for all your datapacks. -The datapack advancements can be everywhere BUT NOT IN THE GLOBAL/ADVANCEMENTS folder or paths from there.

That's the fact. Also, you don't need to put the entity tags in the minecraft datafolder if they already exist in the game.

al-beqaa commented 3 years ago

-The root advancement MUST BE in global/advancements. -The creator advancement can be everywhere but with the creator namespace and always in the same directory for all your datapacks. -The datapack advancements can be everywhere BUT NOT IN THE GLOBAL/ADVANCEMENTS folder or paths from there.

That is exactly what I did. Global root is under global/advancements, creator and datapack advancements are under global/advancements/chromakey.

(And yes, I needed to add the minecraft datafolder tag. I was extending it to include other entities so they would be highlighted by bells.)

UltroGhast commented 3 years ago

that's not exactly what you did. Root and creator are in the right directory. Datapack is not(it is under global/advancements/folder).

Citing again the datapack advancement convention:

You can create this advancement anywhere, as long as you don't pollute /data/global/advancements/folder.

You are modifying a default minecraft data when you can avoid to do it creating your own tag in your own datafolder. Do it or just use the minecraft default data. This is not a convention, but it is useful for compatibility between datapacks.

al-beqaa commented 3 years ago

the convention states:

You can create this advancement anywhere as long as you don't pollute /data/global/advancements/ folder.

not

You can create this advancement anywhere as long as you don't pollute /data/global/advancements/folder

as you seem to think.

and yes, I need to add that entity tag under the minecraft namespace. Again, the #minecraft:raiders tag is used internally by the game, and I need to hook into it to extend that functionality

UltroGhast commented 3 years ago

I talked to EstEarth also, and we both agree the datapack advancement should not be in /data/global/advancement/chromakey, cause it is part of global/advancements. But pratically, this is not a problem for the compatibility.

I shouldn't say that, but ✔️Certified Compatibility(but please don't modify minecraft tags and create your own, also if similar) ✔️Stamp of Quality

You can now use both banners on the datapack page.

al-beqaa commented 3 years ago

Oh, now I think I see what you were saying: anything other than global:root should be under a different namespace. That makes sense.

Keep in mind there are other certified packs doing the same thing though. I would recommend stating that explicitly in the conventions page if it is a factor considered for review.