BloodyMods / ExNihiloCreatio

Ex Nihilo with progression!
MIT License
25 stars 20 forks source link

Hammers not respecting mininglevel #249

Closed winnetrie83 closed 5 years ago

winnetrie83 commented 5 years ago

I changed the hammerregistry to this: https://pastebin.com/egNtDf1Q And i can still mine stone with all hammers After looking to the wiki, i also tried this: https://pastebin.com/iGMKqW3A In both situations the hammer registry is loaded and registered. They work properly except for the mining level. The mining level is not working.

Forge -14.23.5.2838 MC 1.12.2 exnihilocreatio-1.12.2-0.4.jar

Edit: I discovered that every block you register in the hammerRegistry can always be mined with every hammer giving you back the block you were mining. Except for stone, because stone always drops cobblestone. I'm pretty sure this is not intended to work like that. The purpose of this, is to provide some sort of tier, but if you can always mine the block no matter what the mining level is, then that purpose is gone.

SirLyle commented 5 years ago

Except for stone, because stone always drops cobblestone.

That is vanilla behavior whenever you break stone with a non silktouch item.

When you get the block back, what is happening is the breaking of the block is being controlled by vanilla's sense of what a mining level is; when the HammerRegistry then checks for hammer drops it detects none, so it does nothing and you get the vanilla drops. Try registering blocks like obsidian which require higher mining levels and you should see what I mean.

The miningLevel in the HammerRegistry lets you give extra/different outputs when using better hammers, for example you could add an extra iron ore piece with a small chance if using an iron or higher hammer. Actual block breaking is based on the mining level of the block (I believe one of the tweaker mods can be used to edit that).

I'm hesitant to void all drops if the using a hammer on an unregistered block/miningLevel combination since I have visions of angry posts about hammers voiding AE systems or the like.

winnetrie83 commented 5 years ago

But you could implement it to not drop the block if the mining level is not equal or higher. That shouldn't be that hard to implement it. In the blockbreak event you check what the player is holding(wich hammer) and what the mininglevel is of that tool, then compare this with the mininglevel you have given for that block and if the mining level of the tool is not equal or higher then the mining level as the block, cancel the event. I'm more an amateur modder and i do know how to implement this. Ofcourse i can do this with crafttweaker, but i thought it was meant to be a standard feature in your mod

SirLyle commented 5 years ago

But you could implement it to not drop the block if the mining level is not equal or higher.

See my previous concerns about voiding blocks when it is an invalid recipe.

That shouldn't be that hard to implement it. Voiding drops would be easy event.getDrops().clear();, it is a concern with checking if something is cobble versus something which doesn't require a tool versus some modded block with weird behavior versus an inventory block with a nonstandard way of handling its dropped contents (gravestones).

then compare this with the mininglevel you have given for that block

A small complication is that you are not restricted to 1 miningLevel per block, if you wanted you could have cobble give:

cancel the event.

HarvestDropsEvent is not cancel-able. The current behavior of returning the block mined is what happens when you do nothing in the event.

Block breaking is controlled by these two functions (1 and 2) not the harvest drops handler. It might be possible to make those smarter, but is gonna be annoying for the tinkers style hammer.

winnetrie83 commented 5 years ago

I don't see why this is a complication. You still can check for the mininglevel at the current moment and cancel the event if needed. I was talking about the BlockBreakEvent not the HarvestDropsEvent. The BlockBreakEvent is an event that happens when a block is being broken. At that moment you can do the check and cancel it. Doesn't matter how much things i have given to cobble. If the mininglevel is not met, you cancel the event. There is no voiding, i don't understand why you are talking about voiding items. I think you are mistaking BlockBreakEvent for HarvestDropsEvent wich are 2 very different events. Edit: Like this, it wouldn't even mess with tinkers version of the hammers, because you literally check for the tooltype and mininglevel. Here some pseudo code: Hey i am being broken.....does a player do it? yes? Then what Tool is he holding? Aaah a exnihilo hammer....What mining level does it have? 1 you say? Oh srry i just looked in the hammerRegistry file and it says i require mininglevel 2. So you can't break me. interupts the breaking

SirLyle commented 5 years ago

Ah, I misunderstood I thought you were saying modify the existing event and not adding a new one.

interupts [sic] the breaking

proceeds to break vanilla style behavior

I have something which I think is what you want without using a pointless event and without causing surprising non vanilla style interactions.

Edit: also and this.

winnetrie83 commented 5 years ago

Yes i think that code could work. It looks like it, but like i said...I'm more an amateur modder. Yeah i know what pseudocode is :-) I am not native to english and i do not know sometimes how to explain or say certain things. So sometimes it may sound weird. I do not know what you mean with condescending, i translated it, but i'm not sure if i made you angry or... if that is, my apologies. It wasn't meant to be.

winnetrie83 commented 5 years ago

For now, i remove the stone mining from the file, so stone can't be mined at all and i'll let players sieve for pebbles wich can be turned in to cobblestone wich can then be smashed with hammers into gravel. I think that's the best and easiest thing i can do to achieve what i want. Thank you for your time SirLyle. Keep up the good work.

SirLyle commented 5 years ago

The latest beta on curse has these changes.