mc-datapacks / review-tracker

Datapack Review tracker
6 stars 1 forks source link

TCC V0.4: The End and Emerald Update #51

Closed CreeperMagnet closed 3 years ago

CreeperMagnet commented 3 years ago

Project Page: https://thecreeperscode.com/download/

Changelog for this version: https://thecreeperscode.com/development/0-4-changelog/

Good luck.

oOBoomberOo commented 3 years ago

A bounty of $10 has been added to this review, you will be rewarded if you choose to review this datapack.

Keep in mind that I will be expecting a high-quality review with this bounty.

EstEarth202 commented 3 years ago

Certified Compatibility βŒβ“

Stamp of Quality βŒπŸ”†

πŸ’•β”β”β”β”β”γƒ»Stampγƒ»β”β”β”β”β”πŸ’•

Datapack Advancement βœ”οΈ

Datapack Uninstallation βœ”οΈ

Reloading Message πŸ”†

This is still just a part of style guideline it doesn't affect compatibility review.

Ex. tcc\loot_tables\items\amethyst_gemstone.json That I tweaked it down to make it look easy. Code_-_Insiders_K0gXtIdJHW

Global Durability πŸ”† β˜‘οΈ

Some of your custom items also have durability values. You can use this item, depending on your convenience. If let me introduce There is an example like this.

Ex. tcc\loot_tables\items\boomerang.json Code_-_Insiders_OlnYpq5SfH

βš β”β”β”β”β”γƒ»Important γƒ»β”β”β”β”β”βš 

Custom Model ID βœ”οΈβ“

But I have a few questions. Since I found the number 42XXXX and 683XXXX in your Resource pack file, for example

Code_-_Insiders_j4bHGnL3yf Code_-_Insiders_HvI2abaOLa

If I guess you did because of compatibility, if a datapack of hashs and andente is being used with yours datapack?

Global ignoring tag ❌

tcc\functions\block\block_breakers\chopper\break.mcfunction
tcc\functions\block\block_breakers\excavator\break.mcfunction
tcc\functions\block\block_breakers\harvester\break.mcfunction
tcc\functions\block\block_breakers\sifter\break.mcfunction
tcc\functions\block\block_breakers\snipper\break.mcfunction
tcc\functions\block\frostbloom\break.mcfunction

↳ kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill.

tcc\functions\block\jewelry_table\break.mcfunction

↳ as @e & kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill.

tcc\functions\block\nether_reactor\break.mcfunction

↳ kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill.

tcc\functions\block\positional_anchor\break.mcfunction

↳ kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill. except positional_anchor_item

tcc\functions\item\paintbrush\raycast.mcfunction ↳ line 15 Col 67 as @e[dx=0,dz=0,dy=0,type=painting,tag=!global.ignore] It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill. Because I found that this file is outgoing calls of Code_-_Insiders_pj9LQ3aYXM with all kill @s.

❓ tcc\functions\item\riftjuice_bottle\iterate_z.mcfunction This file contains tp @s ~ ~ ~ I think it should have tag=!global.ignore.pos From where I checked, its entry is tcc\advancements\technical\item\drink_riftjuice.json. Because if player has global.ignore.pos from another datapack during that time, there might be a problem. Or if you have any other reason please report.

tcc\functions\item\frostbloom_petals\feed_parrot.mcfunction -> as @e It doesn't have tag=!global.ignore`.

You forgot to remove the tag=!global.ignore from as @p in these files.

tcc\functions\block\jewelry_table\export_slot\0.mcfunction
tcc\functions\block\jewelry_table\export_slot\1.mcfunction
tcc\functions\block\jewelry_table\export_slot\2.mcfunction
tcc\functions\block\jewelry_table\export_slot\3.mcfunction
tcc\functions\block\jewelry_table\export_slot\4.mcfunction

tcc\functions\technical\second_clock.mcfunction ↳ as @e[type=witch,tag=!global.ignore,sort=arbitrary] It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill,tag=!global.ignore.pos.

Ingoing Calls function: Code_-_Insiders_mgDnsHtxDo Because I found this file tcc\functions\entity\witch\switch_to_trader.mcfunction contains tp ~ -1000 ~ & kill @s

tcc\functions\block\block_breakers\chopper\place.mcfunction
tcc\functions\block\block_breakers\excavator\place.mcfunction
tcc\functions\block\block_breakers\harvester\place.mcfunction
tcc\functions\block\block_breakers\sifter\place.mcfunction
tcc\functions\block\block_breakers\snipper\place.mcfunction
tcc\functions\block\geomancer_pillars\explosive\place.mcfunction
tcc\functions\block\geomancer_pillars\normal\place.mcfunction
tcc\functions\block\jewelry_table\place_ew.mcfunction
tcc\functions\block\jewelry_table\place_ns.mcfunction
tcc\functions\block\nether_reactor\place.mcfunction
tcc\functions\block\positional_anchor\place.mcfunction

↳ When these files it is a custom block placing type command. It should have all of this Tags: ["global.ignore", "global.ignore.kill", "global.ignore.pos"] to avoid missing block or change location with kill or tp command.

Namespace Convention βŒβ“

scoreboard objectives add major dummy
scoreboard objectives add minor dummy
scoreboard objectives add patch dummy

Or do you have any reason please let me know.

Shulker Box Loot Table βœ”οΈ

πŸŽ―β”β”β”β”β”γƒ»Other γƒ»β”β”β”β”β”πŸŽ―

Something missing ❓

Bug report 🐞

Tecnical advice πŸ”†

Did you know: fake_player doesn't need to use namespace because it is already inside the objective namespace. tcc.ignore_armor tcc.dummy -> ignore_armor tcc.dummy

πŸ“β”β”β”β”β”γƒ»Noteγƒ»β”β”β”β”β”πŸ“

βœ”οΈ = Followed πŸ”† = Advised to use / Not force ❌ = Doesn't follow ❓ = Doubt / Something missing β˜‘οΈ = Not required/Skip

EstEarth202 commented 3 years ago

If there are any changes other than those mentioned in the review, such as adding files or commands, please add them in the comments.

CreeperMagnet commented 3 years ago

Great. sticky fingers. Reopening.

CreeperMagnet commented 3 years ago

Alright, let's get into the meat and potatoes of this.

For this, since there is a tellraw message from one advancement, even if it is not from load.json, in cases like this with a tellraw, it is counted as well. You already have these in The Creeper's Compendium book. And I think you can use the book to do this instead of tellraw for Help, Not correct version, Or the Resource pack is not installed.

I literally cannot use the book for this. It's also a one-time message that appears to make sure that players have the correct version installed. I'm not spamming the log every reload, so I think this should pass easily.

Common Trait Convention

lmao no ctc is stupid

Global durability

also stupid

Custom Model IDs If I guess you did because of compatibility, if a datapack of hashs and andente is being used with yours datapack?

Yes, this is true. This is why I labelled the folder "tcc_compat" rather than "tcc". As you can notice, I also added sub-namespaces. It shouldn't be of any issue.

global.ignore

global.ignore.kill tcc\functions\block\block_breakers\chopper\break.mcfunction tcc\functions\block\block_breakers\excavator\break.mcfunction tcc\functions\block\block_breakers\harvester\break.mcfunction tcc\functions\block\block_breakers\sifter\break.mcfunction tcc\functions\block\block_breakers\snipper\break.mcfunction tcc\functions\block\frostbloom\break.mcfunction ↳ kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill. tcc\functions\block\jewelry_table\break.mcfunction ↳ as @e & kill @e It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill.

Many of these commands, in their entirety, are variations of kill @e[tag=!global.ignore,limit=1,type=item,distance=..2,nbt={PickupDelay:10s,Item:{id:"minecraft:dropper",tag:{display:{Name:"{\"translate\":\"block.tcc.chopper\"}"}}}}]. These are killing EXTREMELY specifically named dropper items, with my namespaced lang attached. This is not a compatibility issue, and I only added the global.ignores to these commands so my regex match wouldn't go off. Missing global.ignore.kill on these quite literally doesn't matter.

I've added these global.ignore.kills anyways for the sake of bureaucracy.

tcc\functions\item\paintbrush\raycast.mcfunction

I've fixed this file in question. This seems to be me actually missing something, thanks.

tcc\functions\item\riftjuice_bottle\iterate_z.mcfunction This file contains tp @s ~ ~ ~ I think it should have tag=!global.ignore.pos From where I checked, its entry is tcc\advancements\technical\item\drink_riftjuice.json. Because if player has global.ignore.pos from another datapack during that time, there might be a problem. Or if you have any other reason please report.

This is a iterated positioned teleport, which means that I've positioned the execution so that when this tp @s ~ ~ ~ runs, it teleports the player to the positioned execution spot. In this case, it's taking coordinates from a scoreboard, which are gotten from an item's NBT.

You didn't really need to know all that, but I've fixed this anyways.

tcc\functions\item\frostbloom_petals\feed_parrot.mcfunction -> as @e It doesn't have tag=!global.ignore`. You forgot to remove the tag=!global.ignore from as @p in these files. tcc\functions\block\jewelry_table\export_slot\0.mcfunction tcc\functions\block\jewelry_table\export_slot\1.mcfunction tcc\functions\block\jewelry_table\export_slot\2.mcfunction tcc\functions\block\jewelry_table\export_slot\3.mcfunction tcc\functions\block\jewelry_table\export_slot\4.mcfunction

Fixed the parrot case, and I simply forgot to push the export_slot fixes into the current V0.4 repository. Those aren't really an issue, however.

global.ignore.kill + global.ignore.pos tcc\functions\technical\second_clock.mcfunction ↳ as @e[type=witch,tag=!global.ignore,sort=arbitrary] It only has tag=!global.ignore, but doesn't have tag=!global.ignore.kill,tag=!global.ignore.pos.

global.ignore.pos is not relevant to this, but I have added global.ignore.kill. tp @s -1000 doesn't actually do anything position-wise to normal mobs. It's quite literally equivalent to sending them to the void, which is what /kill does. tp @s ~ -1000 ~ Simply kills the mob without any effects such as smoke or loot.

global.ignore + global.ignore.kill + global.ignore.pos tcc\functions\block\block_breakers\chopper\place.mcfunction tcc\functions\block\block_breakers\excavator\place.mcfunction tcc\functions\block\block_breakers\harvester\place.mcfunction tcc\functions\block\block_breakers\sifter\place.mcfunction tcc\functions\block\block_breakers\snipper\place.mcfunction tcc\functions\block\geomancer_pillars\explosive\place.mcfunction tcc\functions\block\geomancer_pillars\normal\place.mcfunction tcc\functions\block\jewelry_table\place_ew.mcfunction tcc\functions\block\jewelry_table\place_ns.mcfunction tcc\functions\block\nether_reactor\place.mcfunction tcc\functions\block\positional_anchor\place.mcfunction ↳ When these files it is a custom block placing type command. It should have all of this Tags: ["global.ignore", "global.ignore.kill", "global.ignore.pos"] to avoid missing block or change location with kill or tp command.

This is not valid. global.ignore is equivalent to all ignore types combined. Boomber says this himself in the original convention doc. There is never going to be a case where I need to add .pos to my armor stands, since no command (that has been properly certified) is going to teleport an entity with global.ignore anyways. Franky, due to this very fact, global.ignore.pos and global.ignore.kill are incredibly useless.

Namespacing

Namespace Convention βŒβ“ Clear The problem is with data\version. I think since this pack belongs to you everything should be in your namespace tcc folder including these objectives. scoreboard objectives add major dummy scoreboard objectives add minor dummy scoreboard objectives add patch dummy

This is due to an idiotic library I use, I've already removed said code.

Other

There are no problems here. But let's me ask to be sure tcc\functions\item\boomerang\pickup_attempt.mcfunction I see here as @e[gamemode=!creative,gamemode=!spectator,distance=..2,sort=arbitrary,scores={tcc.invul_timer=0}] did not specify type=player, but from what I checked. score tcc.invul_timer is only available for player from advancement glant, so here you don't need to specify type=player, am I getting it right?

Yes, that's correct, but also the fact that there's a gamemode argument applied. Normal entities can't have gamemode, nor be checked by it. I'm going to add type=player for further optimization anyways.

tcc\functions\item\boomerang\throw.mcfunction lol πŸ˜‚, I don't think you will still have to use old teleport command. teleport @e

This is actually the new version of tp. I used it in this situation a while ago due to differences in the two commands during 1.13 development stages, but you don't need to worry about that.

Clear When I check the tp @s command at tcc\functions\entity\iceologer\spawn.mcfunction. I can't find the tag tcc.iceologer_replace When was it added to the pillager, would you please tell me?

Those pillagers are spawned naturally within a structure, which of course, is a .nbt file you can't easily read.

Tecnical advice πŸ”† Did you know: fake_player doesn't need to use namespace because it is already inside the objective namespace. tcc.ignore_armor tcc.dummy -> ignore_armor tcc.dummy

Yes, I'm well aware of this. This is simply for my own peace of mind so I can easily identify which pack I'm working with at the moment. It also looks a lot neater.

So, to summarize:

Chat message

This is a one-time chat message, that is one line, that is also essential for checking the player's resource pack, that I cannot place anywhere else. This doesn't touch the "no reload tellraw spam" convention.

ctc and durability

No.

global.ignore

There's a whole lot of places you improperly identified where certain global.ignores downright didn't apply, and I think you should really read up on what it's supposed to do. However, there were 3 small cases you did find that actually cause compatibility issues, and those I have already fixed.

The majority of the rest of them that were "problems", I've added the tags just for the sake of bureaucracy

Namespace and CustomModelData

The versions library I was using has been removed, so that lack of namespacing is fixed.

The CustomModelData usage was for compatibility that I've already discussed with the owners of their respective packs.

Thank you, @EstEarth202, again, for reviewing this massive undertaking of a pack!

EstEarth202 commented 3 years ago

Certified Compatibility βœ”οΈ

Style Guideline Followed πŸ’•

Final comment: It's a cool pack that's enjoyable to play, This is one of the best sample standalone pack to guideline about file directories structure, optimization & namespace. LMAO Datapack reviewer @EstEarth202. :thumbsup:

https://www.planetminecraft.com/data-pack/the-creeper-s-code/ https://thecreeperscode.com/download/