Minecraftforge: [1.13.2] Set harvest level from other mod's blocks

[tyra314]

I recently looked into porting my mod ToolProgression to Minecraft 1.13.2. The whole point of this mod is to allow modpack makers to overwrite the harvest levels and tools classes of items and blocks in one place.

But I quickly realized that essential methods were removed. These are:

• Block.setHarvestLevel()
• Item.setHarvestLevel()

I found this old PR: #5040 where it was shortly discussed that it's not intended behavior to be able to set harvest levels from other classes. But this, in the end, means the death sentence for all mods (or features of mods) trying to do that, unless I'm overlooking something here.

Therefore, it may also significantly hurt the experiences an "expert mode" modpack might be able to deliver, as those packs tend to tweak harvest levels a lot. The problem is if only the mod itself should be setting harvest levels, they are the ones in charge of also providing a config options to allow pack makers to overwrite those levels. I'm not aware of any mod, who provides harvest level config options for its blocks, some do for items though.

So can you please bring some light to this issue - what would be a feasible path forward to keep these mods alive?

ps: The same basically applies to every other removed setter of those two classes.

[Shadows-of-Fire]

You may be able to achieve the desired result via HarvestDropsEvent and PlayerEvent.BreakSpeed. Vanilla has changed those fields into protected final fields without setters. If nothing else, you'll need to use reflection or an AT to mess with them.
It is unlikely forge re-introduces setters for these fields, however.

[tyra314]

Thank you for your thoughts, but I'm not seeing a solution here, yet.

You may be able to achieve the desired result via HarvestDropsEvent and PlayerEvent.BreakSpeed.

Circumventing the whole system isn't a good idea at all. Using these events to get a consistent behavior was already bad enough with having the setters. It can only get worse. If it hasn't changed, HarvestDropsEvent is only triggered, when loot is actually dropped, not when blocks are destroyed. And the PlayerEvent.BreakSpeed needs fiddling to reestablish the correct break speed in the case of you try to harvest a not harvestable block.Additionally, this event only allows you to prevent block harvesting, it won't allow you to harvest blocks and get drops, if it wasn't allowed according to the original harvest level. I also used to use BlockEvent.BreakEvent to trigger my own drops, which was so evil and messy. It needed figuring out the correct priority, so it gets called before or after certain mods. It caused disappearing blocks and probably drop dupes, because you don't have all the information you'd need to decide what to do. For example, wrenching blocks is a nightmare. This whole code was and will not be maintainable and would cause more harm than good.

Vanilla has changed those fields into protected final fields without setters. If nothing else, you'll need to use reflection or an AT to mess with them.

Well, every mod could end up with its own implementation of the getters and thus making it a nightmare to support other mods at all. Thus, somehow invalidating my mod in the first place.

It is unlikely forge re-introduces setters for these fields, however.

Too bad. The only thing I'm seeing right now, is to coremod into ForgeHooks.canHarvestBlock and completly replacing the vanilla/forge harvest checks :(

[Shadows-of-Fire]

Every mod could've ended up with their own implementation of the getters in 1.12 or any previous version as well, that shouldn't really be a concern, it's very unlikely people do that.

[tyra314]

Unfortunatly, it's not unlikely. I'm sure ThermalFoundation did it. And IIRC TinkersConstruct also did it. The thing is, with having a setter, I can argue that the mod's dev(s) should add a proper setter when they overwrite a getter. When there is no setter in the first place, I'm screwed in that regard.

[LexManos]

As I said in the other thread. Setting other people's tools and harvest levels was never the intention of the original edit. It was just a side effect of how it was implemented.

I am open to other ideas tho, perhaps a event or a data object that holds a single 'Master Harvest management' system. Similar to the Permissions system. I can see the use for ONE utility mod like this to be installed at a time. But Not for untracked overwriting of anyone's levels at anytime you want.

[tyra314]

I am open to other ideas tho, perhaps a event or a data object that holds a single 'Master Harvest management' system.

Judging only from my limited understanding of the code structure in 1.12/1.11, I'd say the missing piece would be an event, which is triggered right before the block is destroyed and the outcome of the event determines if there will be drops or not. As strived shortly above, the HarvestDropsEvent lacks this; It is only triggered if the vanilla code determined that there will be drops, so you can just prevent drops.

Having such a more generic HarvestDropsEvent and the BreakSpeedEvent should get you somewhere, even though I'm still not too fond of working completely around the default system 😞 On the bright sight, it would allow a total rethinking of what canPlayerHarvest could mean.

I can see the use for ONE utility mod like this to be instaedll at a time.

I totally agree, even just thinking about the countless things that inevitably go wrong with multiple such mods in one pack gives me nightmare fuel for weeks.

*edit: I also remembered that @darkhax had a similar problem in OreStages as this mod fake blocks to be something else and thus had to fake the harvest level and tool class of some other blocks. This use case might be part of the solution. too.

[Darkhax]

Yes, the BreakSpeedEvent can be used to mimic this behavior, however in my experience this approach is far from reliable and frequently has conflicts with other mods. In the case of my OreStages mod, what I would really need is an event which allows the final result for the break speed to be calculated, or additional events which allow for the variables involved to be changed.

[EmeraldLangtree]

Event sounds better. Mods may be interested in different types of block. But also easier to get wrong. I think client and server must have the same data for this to work?

[LexManos]

An event for this will not be accepted. As stated the only logical thing I can see this being a use for is total overhaul mods. So a SINGLE provider with a simple config is the way it should go. If people wish this proceeded they must provide context and discussion. Until then, going to mark this as something for me to ignore until someone tells me there has been a update.

[tyra314]

I don't think a single provider will work out in the end. It sounds good at first, but it will not stand the test of time. I see a few issues with that approach:

1. Where do you draw the boundaries for the provider? Does it allow to tweak only harvest levels? Hardness? Tool efficiency? What if two mods want to tweak different subsets of the provided interface?
2. What if you have two different "overhaul" mods with different aims. This may sound silly at first, but consider something like my ToolProgression and OreStages. Both are overhaul mods, but with a completely different intent. They do not interfere with each other and sure you could have both in one mod pack. (Btw. I'm only picking two examples here, I'd bet there are even more mods in the same vein)
3. More generic mods like CraftTweaker will probably occupy the provider just for good measure, because there may or may not be a script using some interface of the provider. This would kill all other "overhaul" mods right from the start, because if you have to choose between CraftTweaker and $RANDOM_MOD the decision is clear. Heck, even I would choose CraftTweaker over my own mod.

I'm sorry, but one single provider isn't the solution either.

[LexManos]

1) That is up for the community to decide. As I said you guys need to discuss what things you need access to.
1b) Then you're SOL in that theoretical setup, one provider wins just like Permissions.
2) Then one of them would 'win' and be the one to manage the global overrides. Ideally this would be chosen with a config. And gracefully handeled by both mods when they don't win.
3) Then CraftTweaker becomes the provider. I don't see a issue with this. Your complaint about it killing over 'overhaul' mods is invalid for two reasons. 1) CraftTweaker/MineTweaker already does that, and 2) Again the 'winner' should be configurable and any mod using this system needs to gracefully handle NOT being the winner.

[Darkhax]

CraftTweaker definitely does not override core systems of the game in a way that breaks compatibility with other mods. Especially not in the context of tyra's response.

As for this issue, the core problem is that there are already many mods which modify various aspects of the block breaking algorithm, which has lead to conflicts between mods and continues to do so. To give some examples of this

• Ore Stages - Player based flag. If you don't have the flag block A acts as block B.
• Tool Progression - Configures which items are able to harvest a block.
• BlockTweaker - Allows CraftTweaker scripts to change the base harvest level, and hardness properties of blocks.

Each of those mods have completely distinct goals, are conceptually 100% compatible, and all want access to modifying the same set of data. Some conceptual examples would be things like potion effects, enchantments, and baubles which have the ability to increase or decrease the mining level of the player. Another would be something like the mining dimension but all the ore blocks have a higher harvest level.

I fully agree that the two methods mentioned should not be added back, however this is definitely a lacking area and I feel some of these events should be revisited for 1.13. The HarvestCheck event is a prime example of this. It is only fired if the player breaks a block without an item, or if the item has a negative tool level. It is also ignored all together if the block has a "requiresNoToolFlag". This behavior does not reflect the behavior described in the Javadoc of the event, and also seems extremely specific. Ideally this event would be replaced with one or more events which allowed the harvest level of the player, the required harvest level of the block, and the actual result of the method to be modified appropriately. This would completely solve all of the issues I have with the current system in place. The event also has some smaller issues, like it has its own boolean for being canceled instead of using the built in cancellation system.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.