Fixed some issues - Githubissues

ghost commented 4 years ago

Fixed some issues. Each commit should provide the series of changes I've made 😄

puggan commented 4 years ago

Thanks. Think I want some of thus on my master branch too.

puggan commented 4 years ago

195a46e tags/blocks/walls.json is needed for the walls to act like walls togehter with other walls. tags/blocks/stairs.json is required for corners

puggan commented 4 years ago

959f516 Added an logo.png, but refers to it as icon.png

puggan commented 4 years ago

115ae54 I see no difference between having and not having breakByTool(). Can you give an example difference?

puggan commented 4 years ago

Cherry-picked to my master branch:

96e229a Added the logo and expanded the contact section
bd05ca2 Added the black brick wall
292fb08 Fixed language issue
7147bc5 Added a LICENSE that will be bundled in with the mod

Cherry-picked for the fabric branch:

82fe2d0 When building a new release, all JSON files are minified, saving space for the new jar file
a866492 Added the logo and expanded the contact section
e9282fb Forgot to commit the misc. gradle stuff :P
165445f Removed unnecessary comments and added Mod Menu and REI to the mod development workspace
315deb1 Don't need to have a pack.mcmeta
e6e01c6 This is Spart- I mean Fabric!

Skipped:

115ae54 Now the blocks are breakable with a wooden pickaxe like regular bricks
541c096 Simplified some code
f72ea82 Shouldn't add the tag we are using for our recipes because a soft error occurs when an already existing entry (like "minecraft:bricks" which is in the tags we are using for our recipes) is added to a tag
195a46e Not needed for recipes

ghost commented 4 years ago

195a46e tags/blocks/walls.json is needed for the walls to act like walls together with other walls. tags/blocks/stairs.json is required for corners

Sorry for asking but isn't that for the Minecraft namespace? I deleted the block tags of the mod's namespace but I added all the appropriate blocks for the appropriate tags. Or is there something I'm missing/not getting?

ghost commented 4 years ago

959f516 Added an logo.png, but refers to it as icon.png

Sorry for the mismatch! Wasn't paying attention when committing the files.

ghost commented 4 years ago

115ae54 I see no difference between having and not having breakByTool(). Can you give an example difference?

Whenever you make a block in Fabric and you want to make a block that's only breakable by a specific tool, you add

.breakByTool(FabricToolTags.WHATEVER_TOOL_YOU_WANT, miningLevel)

You can see this being used in the following mods (but ofc it's not limited to these mods):

puggan commented 4 years ago

195a46e tags/blocks/walls.json is needed for the walls to act like walls together with other walls. tags/blocks/stairs.json is required for corners

Sorry for asking but isn't that for the Minecraft namespace? I deleted the block tags of the mod's namespace but I added all the appropriate blocks for the appropriate tags. Or is there something I'm missing/not getting?

I added my blocks to my tag, and added a reference to my tag in the minecraft namespace:

https://github.com/puggan/mc-colorbricks/blob/5ed03d9588a676bb729650caa0a43b00ad7b5166/src/main/resources/data/minecraft/tags/blocks/walls.json#L1-L6

The mention commit just removed them, I later noticed that you re-added all blocks in the minecraft namespace in a later commit. Is there a performance difference with using an intermediate tag? Or what is the gain in removing the intermediate tag?

puggan commented 4 years ago

115ae54 I see no difference between having and not having breakByTool(). Can you give an example difference?

Whenever you make a block in Fabric and you want to make a block that's only breakable by a specific tool, you add
.breakByTool(FabricToolTags.WHATEVER_TOOL_YOU_WANT, miningLevel)
You can see this being used in the following mods (but ofc it's not limited to these mods):
* [Gubbins](https://github.com/Lemonszz/gubbins/blob/9694e31bd6d8c0250c958d7c66a76814ecc6028c/src/main/java/party/lemons/gubbins/init/GubbinsBlocks.java#L16)

* [Time Travel Mod](https://github.com/rdvdev2/TimeTravelMod-Fabric/blob/c35c911122e26c4db46cd2785a6d8653f612e4a1/src/main/java/com/rdvdev2/TimeTravelMod/ModBlocks.java#L29)

* [Astromine](https://github.com/Chainmail-Studios/Astromine/blob/5848a5f406b2382734e94c858c0b73796ed60a71/src/main/java/com/github/chainmailstudios/astromine/registry/AstromineBlocks.java#L106)

I want my blocks to act as the normal bricks-blocks in the game. And normal bricks isn't "only breakable by a specific tool". You can break a brick-block by hand, it slow, but it breaks. If you use a pick-axe, it's breaks faster. Both is also already true about my bricks-blocks.

My understanding is that you can use breakByTool() to override the default behavior, By default pickaxe is useful on STONE, METAL and REPAIR_STATION see net.minecraft.item.PickaxeItem#isEffectiveOn() So by using STONE as base, it gets the right default behavior.

In all 3 exemples you gave, the tool-level is set to higher then zero, that way a wooden pick-axe is'n efficent on that block, so there it make sens, as they don't want the default from STONE.

ghost commented 4 years ago

I added my blocks to my tag, and added a reference to my tag in the minecraft namespace:

https://github.com/puggan/mc-colorbricks/blob/5ed03d9588a676bb729650caa0a43b00ad7b5166/src/main/resources/data/minecraft/tags/blocks/walls.json#L1-L6

The mention commit just removed them, I later noticed that you re-added all blocks in the minecraft namespace in a later commit. Is there a performance difference with using an intermediate tag? Or what is the gain in removing the intermediate tag?

A soft error occurs in Mojang's code when two entries exist: in this case, it will be the "minecraft:brick_wall" entry. It shows up in the debug log as an error (though it honestly should be logged as a warning imo) and I thought it would be best to remove the need of an intermediary tag. It doesn't actually matter much in terms of performance nor terms of functionality. I decided that it perhaps would be the best to avoid the error.

ghost commented 4 years ago

I want my blocks to act as the normal bricks-blocks in the game. And normal bricks isn't "only breakable by a specific tool". You can break a brick-block by hand, it slow, but it breaks. If you use a pick-axe, it's breaks faster. Both is also already true about my bricks-blocks.

My understanding is that you can use breakByTool() to override the default behavior, By default pickaxe is useful on STONE, METAL and REPAIR_STATION see net.minecraft.item.PickaxeItem#isEffectiveOn() So by using STONE as base, it gets the right default behavior.

In all 3 exemples you gave, the tool-level is set to higher then zero, that way a wooden pick-axe is'n efficent on that block, so there it make sens, as they don't want the default from STONE.

.breakByTool(FabricToolTags.WHATEVER_TOOL_YOU_WANT, miningLevel)

signifies that the pickaxe is the best fit for the job instead of, let's say, a sword. A diamond sword is faster at breaking a brick then using your hand (by a very small margin) but a diamond sword will get more damage per brick broken than a diamond pickaxe. It also specifies the mining level, which is important for mods that adds custom pickaxes so you can break bricks with those mods' pickaxes.

puggan commented 4 years ago

signifies that the pick-axe is the best fit for the job instead of, let's say, a sword.

Setting the material to STONE already do that.

It also specifies the mining level, which is important for mods that adds custom pickaxes so you can break bricks with those mods' pickaxes.

But as your code sets it to zero, it's no different than the vanilla fallback value for STONE.

A diamond sword is faster at breaking a brick then using your hand (by a very small margin)

A Sword have a speed-bonus 15x on CobWeb, and 1.5x on Plants and Leaves. It has no speed-bonus on other blocks.

but a diamond sword will get more damage per brick broken than a diamond pick-axe.

Breaking a block with a sword cost 2 durability, while Breaking a block with a pick-axe cost 1 durability. This have nothing to do with the block, it's just an effect of using a weapon as a tool gives extra penalties. The durability is omitted if the block breaks instantly, for both swords and pickaxes, but that's not the case for Bricks.

ghost commented 4 years ago

Sorry for making this so unnecessary long because of my lack of understanding! I didn't realize that was the case and still stood to my point even though looking back on it, you were right.

ghost commented 4 years ago

Should I close this PR?

puggan commented 4 years ago

Sorry for making this so unnecessary long because of my lack of understanding! I didn't realize that was the case and still stood to my point even though looking back on it, you were right.

No worry, thanks for all the feedback, most of your commits are implemented in the 0.4.0-fabric release.

puggan / mc-colorbricks

Fixed some issues #2