mc-datapacks / review-tracker

Datapack Review tracker
6 stars 1 forks source link

Better Phantoms #72

Open al-beqaa opened 3 years ago

al-beqaa commented 3 years ago

Project Page: https://datapackcenter.com/projects/better-phantoms.294/

Repository: https://github.com/ChromaKey81/Better-Phantoms

EstEarth202 commented 2 years ago

So in order not to waste time, I have fixed it for you, in fork To reduce explanations that may be lengthy and communication may be inaccurate. So I'm doing this to make it easier. If you don't understand, feel free to ask me.

I will explain to you at each point why to do so.

Since the datapack advancement directories are not valid, I fixed them as I did. Here, I understand why you put files like this. It's from TCC pack that use Standalone Datapack, but in your case you don't have a single datapack. root node /data/global/advancements/root.json namespace(player_head) node/data/global/advancements/<namespace>.json datapack node "You can create this advancement anywhere, as long as you don't pollute /data/global/advancements/folder. " Usually it is placed at data\chromakey\advancements\betterphantoms.json

Basically, using a namespace as a datapack name doesn't seem like a problem. But if other creators come up with the same name, it can be a problem in the future. Especially with tag namespace or nbt namespace. And here's a little explanation from my experience.

Issue: Some packs use the datapack_name as their namespace.

Sub Issue: If you follow Datapack Advancement with defines 3 advancement nodes: Root, Namespace and Datapack.

And because datapack node must not be in /data/global/advancements/folder.

Causing the need to take this file to /data/<namespace>/advancements/

It became like this /data/<datapack_name>/advancements/<datapack_name>.json

You might ask me if I use my name instead of datapack name. You might have this kind of problem.

Your pack1 "chromakey:load"
Your pack2 "chromakey:load",

But that's not a problem if you do this. chromakey/functions/betterphantoms/

data
 ├ <your_name>
 │    ├ advancements
 │    │   ├ <datapack_name>
 │    │   │   ├ <event>
 │    │   │   │   ╰ <player_hurt_entity>.json
 │    │   │   ╰ <place>
 │    │   │       ╰ <place_block>.json
 │    │   ╰ <datapack_name>.json
 │    ├ functions
 │    │   ├ <datapack_name>
 │    │   │   ├ <folder function groups>
 │    │   │   ├ main.mcfunction
 │    │   │   ├ setup.mcfunction
 │    │   │   ╰ uninstall.function
 │    │   ╰ <database, random & etc.>
 │    ├ loot_tables
 │    │   ╰ <datapack_name>
 │    │       ╰ <loot folder groups item, block>
 │    │           ╰ <...>.json
 │    ├ predicates
 │    │   ╰ <datapack_name>
 │    │       ├ <folder predicate groups>
 │    │       ╰ <...>.json
 │    ├ recipes
 │    │   ╰ <datapack_name>
 │    │       ╰ <...>.json
 │    ╰ tags
 │        ╰  <items>
 │            ╰ <datapack_name>
 │                ╰ <...>.json
 ├ <global>
 │    ╰ advancements
 │        ├ <your_name>.json
 │        ╰ root.json
 ╰ <minecraft>

Solution

Fact: It's creator name.

data
 ├ <namespace> <- It doesn't make sense to set it as datapack_name.
 │    ╰ advancements         
 │        ╰ <datapack_name>.json
 ├ <global>
 │    ╰ advancements
 │        ├ <namespace>.json <- "This is creator_head" * That's why
 │        ╰ root.json

Because: If creator plan to write more datapacks, it will be very helpful when you work on workspace.

image

Result: <creator_name>:<datapack_name> When there are multiple packs, the name on front is to identify who created it and next it to tell what datapack it is.

And that's why I have to change the namespace to your name.

Because it works the same as tick.json, and the tick.json file will remain active if no datapack disable. And it's easy to uninstall too, just schedule clear chromakey:betterphantoms/tick. Because if you use datapack disable "file/betterphantoms" If the user installs a .zip, it won't work. And the file name must match as well. Even if you can do like this

datapack disable "file/betterphantoms"
datapack disable "file/betterphantoms.zip"

But if file name is betterphantoms_v1.0.0.zip. Either install it as a .zip or extract to folder it won't work.

<name>.zip -> /data
<name>/data

It's a headache to do this, which I see as unnecessary So I chose the easier way.

The fork review I fixed and a few other fixed issues for you is the version that has certified + style guideline. If you don't have any questions or issues, you can merge pull request and let me know in the comments to close this review. Or, as always, you can reject this as you wish. And I'll close the review for you. Or you can fix it yourself. Let's just think I'm just giving you a fixed example.

Hope this will be helpful to you, more or less, as a Datapack Reviewer.