Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hard to break certain blocks #628

Closed
musznik opened this issue Sep 24, 2015 · 57 comments
Closed

Hard to break certain blocks #628

musznik opened this issue Sep 24, 2015 · 57 comments

Comments

@musznik
Copy link

musznik commented Sep 24, 2015

Its extremly hard to break any : stone bricks or stairs even with max overclock

ModularPowersuits-0.11.0.281.jar

@VladimirMangos
Copy link

I confirm: For some blocks ModularPowersuits tool not recognized as pickaxe.
Waila tooltip show missing required pickaxe tool for block.

@Pinkbyte
Copy link
Contributor

Confirming, but with Axe and Binnie Mods trees. It seems that wood in that mod is somehow checking that class of tool that tries to break it is a child class of some axe.

Why i think so? Because Omnitools from Electro-Magic tools and Axe of Stream from Thaumcraft works fine.

Unfortunately i can not prove my words, because Binnie Mods is closed-source :-(

About pickaxe troubles, i use this workaround - Pinkbyte/MachineMusePowersuits@499703857d7328754652da6bbc99c74f78fc8aa7

Yeah, i know, this code does not make much sense, but for some reason - it works :-)

@MachineMuse
Copy link
Owner

it does kinda make sense - there are a billion ways to check if a block can be mined and it's hard to tell where you're supposed to hook in in order to make it work.

In any case, I changed the code to use the one you suggested. Hopefully it will work now. I currently don't have a computer I can test it with sadly, so please let me know!

@musznik
Copy link
Author

musznik commented Oct 6, 2015

Fixed in 0.11.0.282, thanks Pinkbyte & MachineMuse

@musznik musznik closed this as completed Oct 6, 2015
@musznik musznik reopened this Oct 7, 2015
@musznik
Copy link
Author

musznik commented Oct 7, 2015

forge : 10.13.4.1517
MPS: 0.11.0.282

hmm, it's fixed for bricks or stone brick stairs but not for wooden fence or wooden stairs (full overclock + diamond drill)

@VladimirMangos
Copy link

Code changes applied to pickaxe case, maybe need similar changes for axe case also

@MachineMuse
Copy link
Owner

ok, I probably should have done this in the first place but I changed those around as well. Should be good?

@VladimirMangos
Copy link

I am personally no has early problems with axe case. So can't test it. But after last build update I see problem with dig dirt. Now speed decrease to empty hand dig speed.

@CompeteToDefeat
Copy link

I can't get the shovel to mine things like dirt, gravel or sand as expected. Regardless of overclocking or toggling the shovel on/off it mines them the same speed as using your fist.

Tested with:
Forge 10.13.4.1492
Numina 0.4.0.131
MPS 0.11.0.283

@MtnDewster
Copy link

I'm having issues with the shovel and axe not digging/chopping at the rates they should be. With full overclock they work just as fast as your hands. I have tried making another power tool and only installing the axe module into it and it still doesn't fix the problem.

Tested with:
Forge 10.13.4.1492
Numina 0.4.0.131
MPS 0.11.0.283

@bakaie
Copy link

bakaie commented Nov 12, 2015

Same issue as stated above. Upgraded to MPS 0.11.0.283 and now pick/shovel/axe are digging at normal open had speed. It you try and use the pick on anything above stone level it will destroy the item as if it was done with an open hand.

@Pinkbyte
Copy link
Contributor

Meh, it seems that blind applying for pickaxe to axe and shovel just broke those modules(though i did not check it myself yet)

@XiloTheOdd
Copy link

I'm on MPS's latest build 284, all vanilla and mod blocks treat the power fist as tooled but minimum overclock setting still. no overclocking seems to modify anything except the energy drained per block. it otherwise does mine block appropriately.

@VladimirMangos
Copy link

In my case with 284 power fist stop mine many vinilla/mods stone/brick blocks that required pickaxe.
State back to before 282 way work. But dig dirt with correct speed.

@Alaberti
Copy link

Alaberti commented Dec 8, 2015

Running build 290 and seeing this issue on a dedicated server. I have a lot of users reporting the same thing, but I don't know what to look for in logs or anything to provide you with. Is there anything we can help you do to diagnose this problem?

@binnesman
Copy link

I'm also having the issue with shovel and axe modules not functioning at all. mps build 290, numina build 131. Went back to build 281 to restore functionality.

@VladimirMangos
Copy link

I can confirm that in 290 problem reversed one more time: now pickaxe case work but shovel stop working. I see round moving from pickaxe to shovel/axe and back with new builds. (

@lehjr
Copy link
Collaborator

lehjr commented Dec 10, 2015

If the old code worked for some things while the new code only worked for others, then the easiest solution would be to combine the 2 into something like

Edit: like this (several edits later)
public static boolean harvestCheck(ItemStack stack, Block block, int meta, EntityPlayer player) { if (Items.iron_pickaxe.canHarvestBlock(block, stack)|| (ForgeHooks.canToolHarvestBlock(block, meta, ironShovel))) { if (ElectricItemUtils.getPlayerEnergy(player) > ModuleManager.computeModularProperty(stack, PICKAXE_ENERGY_CONSUMPTION)) { return true; } } return false; }

@lehjr
Copy link
Collaborator

lehjr commented Dec 10, 2015

weird, the wooden stairs are still an issue.

@eyeonus
Copy link
Contributor

eyeonus commented Dec 10, 2015

Is Tinker's Construct open source? If so, maybe look at how they do it, since I've never had a problem like this with tools from that mod.

@lehjr
Copy link
Collaborator

lehjr commented Dec 10, 2015

observations:
the troubled blocks return null for block.getHarvestTool(meta) and return -1 for block.getHarvestLevel(meta) but do have valid materials.

@lehjr
Copy link
Collaborator

lehjr commented Dec 12, 2015

I added a material based check. It needs some testing and probably some fine tuning before I submit a P.R.: lehjr@e8bf541

Test build here: https://github.com/lehjr/MachineMusePowersuits/releases/tag/290a

@binnesman
Copy link

@lehjr Tested it just now, stone/wood stairs are working fine, as well as digging dirt/sand. I did notice power glove on action bar had a grey background that isn't present on 290

@lehjr
Copy link
Collaborator

lehjr commented Dec 12, 2015

@binnesman Thanks for testing. Weird though, as there shouldn't be any visual changes.

MachineMuse added a commit that referenced this issue Dec 14, 2015
@lehjr
Copy link
Collaborator

lehjr commented Dec 19, 2015

Looks like the stuff listed here has been fixed, however, I do have a couple new items, namely tracks and things like EnderIO machines, maybe more. ATM, it's actually faster to break a track barehanded, and EnderIO machines can be broken down with a stone pickaxe, though the preferred tool is a wrench.

@eyeonus
Copy link
Contributor

eyeonus commented Dec 19, 2015

Does the omniwrench module not work on EnderIO stuff?

@bakaie
Copy link

bakaie commented Dec 19, 2015

Yep, i just updated my server to the new version. Dirt and things like
that work at speed, but stone, wood, and other vanilla blocks are slower
then a stone pick. A bunch of the ender IO and some other blocks are
really slow. The OmniWrench has not worked with Ender IO in several
version.

On Sat, Dec 19, 2015 at 1:00 PM, lehjr notifications@github.com wrote:

Either way, the Power Fist with the pickaxe module installed should be as
fast as an iron pickaxe and it isn't, at least in this case. And as I said,
it's not just EnderIO blocks, but even vanilla tracks as well


Reply to this email directly or view it on GitHub
#628 (comment)
.

Thanks
Brian

@DoomRater
Copy link

So glad to see some activity on this issue since the last time it was raised up and closed. Since there are a lot of ways to check if a block is supposed to be mineable, we may need to check all of them. I know that there were versions where this worked just fine on all blocks, I believe back in a 1.4 or 1.5 version of Minecraft, but obviously Forge has changed since then as well.

@MachineMuse
Copy link
Owner

yeah, a lot of different hooks were added since then, and some mods may or may not have been updated to use any of them. So it's quite frustrating :P

@lehjr
Copy link
Collaborator

lehjr commented Dec 21, 2015

I've been experimenting with different solutions, none of them really produce the results I'm looking for, a consistent speed for a breaking a stair and a block of the same type, and to be able to break blocks at the same speed as an Iron pickaxe or axe. I don't know if it's possible, but that's what I'm currently looking at.

@eyeonus
Copy link
Contributor

eyeonus commented Dec 21, 2015

Maybe look at how Tinker's Construct does it? I've never had problems with the tools from that mod.

@lehjr
Copy link
Collaborator

lehjr commented Dec 21, 2015

Thanks, but I already looked at it. It's kind of a wall of code. I also looked at the Atomic Disassembler from Mekanism, but that just breaks all blocks not bedrock. The trick is to be able to fix the problem while working in the confines of the modular framework. Each module has to be limited to breaking the type of block that the vanilla tool of that type can break. It would probably be almost effortless to just create a "Paxel" type module that handles all blocks in one module. I just don't really think it fits well with the theme of MPS, even if it had tiered upgrades.

@MachineMuse
Copy link
Owner

Exactly. To do it the way TConstruct or Mekanism does it, we would have to make multiple 'power tool' items - one for each permutation of harvest levels & types. I decided that was too hacky even for me, and so fought hard to get hooks in Forge to allow a given item's harvest level/type to vary based on the metadata of the item (mDiyo pushed for this too, but only for the harvest level - he still had different item IDs for different harvest types). But only some people had written their code in a way that could be used by the new hooks, while others had overridden other methods and thus ignored them; then the Forge Event system came along and resulted in another set of hooks which could be overridden, and it is all quite a big mess now which only MPS apparently suffers from (everyone else just works around it by having static item harvest types or doesn't need to care).

@bakaie
Copy link

bakaie commented Dec 21, 2015

Draconic Evolution has some tools that can be used on more then one type of
material even at the same time. Like the staff of power, it can dig
through anything in one click, dirt stone, ore. I wonder if that may be
more along the line you are trying to get to, may be worth a look anyway.

On Mon, Dec 21, 2015 at 3:42 PM, Claire notifications@github.com wrote:

Exactly. To do it the way TConstruct or Mekanism does it, we would have to
make multiple 'power tool' items - one for each permutation of harvest
levels & types. I decided that was too hacky even for me, and so fought
hard to get hooks in Forge to allow a given item's harvest level/type to
vary based on the metadata of the item (mDiyo pushed for this too, but only
for the harvest level - he still had different item IDs for different
harvest types). But only some people had written their code in a way that
could be used by the new hooks, while others had overridden other methods
and thus ignored them; then the Forge Event system came along and resulted
in another set of hooks which could be overridden, and it is all quite a
big mess now which only MPS apparently suffers from (everyone else just
works around it by having static item harvest types or doesn't need to
care).


Reply to this email directly or view it on GitHub
#628 (comment)
.

Thanks
Brian

@MachineMuse
Copy link
Owner

No, it is not. The hard part is that the power tool varies what types of blocks it can mine, depending on what modules are installed. Making a tool that can just mine any block is easy.

@Pinkbyte
Copy link
Contributor

I understand that it would be a lot of code(and it will possibly break class hierarchy), but how about to inherit Axe, Pickaxe and so on classes in main PowerTool class and then - override apropriate methods(called on use), which, in turn, will check if upgrade for mining(Axe, Pickaxe) is installed or not.

Yeah, as i said before - code will be ugly :-/

But in case when many mods check if current tool class is inherited from Foo(and when they do not provide any API to override this behaviour) i think that it would be the best option.

@MachineMuse
Copy link
Owner

Java doesn't support multiple inheritance. Scala emulates it with mixins but the ultimate result is not truly multi-inherited and the mixins must still be traits (interfaces).

@Pinkbyte
Copy link
Contributor

Hm... Let's take some examples. There is a mod, called Electro-Magic-Tools. There are some tools in it, called OmniTools, which can act both as Axe and Pickaxe. Let's see how it's working:

https://github.com/TeamAmeriFrance/Electro-Magic-Tools/blob/master/src/main/java/tombenpotter/emt/common/modules/ic2/items/tools/omnitools/ItemOmnitoolIron.java

So, you are right. IronOmniTools class extends only ItemPickaxe.

No ideas then :-(

I doubt that moving all checking code in one function will improve anything. But it will surely degrade modularity of code.

@MachineMuse
Copy link
Owner

The hard part is that the power tool varies what types of blocks it can mine, depending on what modules are installed.

@lehjr
Copy link
Collaborator

lehjr commented Dec 23, 2015

The biggest issue in the current setup is trying to emulate the equivalent vanilla tool. What makes it hard is there isn't a consistent way to check if each of those tools can break a certain block. For instance, not all blocks have a default harvest tool set, blocks like vanilla stairs return null for a default tool.
Adding to that:


and

which should be functionally equivalent, should return true for a vanilla oak tree, should, but they don't, nor do the same functions work with dirt or any diggable block when replacing "Items.iron_axe." with "Items.iron_shovel."

Mekanism's Atomic Disassembler just returns true for every block but bedrock:
https://github.com/aidancbrady/Mekanism/blob/4040a57a16b5a1f68a03810816acb69e9f3dc374/src/main/java/mekanism/common/item/ItemAtomicDisassembler.java#L43

And that's great for what that tool is, but it isn't modular.

As for MPS, I was messing around with overriding getToolClasses from inside of ItemPowerFist and early testing seems to produce some interesting results, though I haven't had a chance to thoroughly test everything or change it based on installed modules.

override def getToolClasses(itemStack: ItemStack): ImmutableSet[String] = { ImmutableSet.of("pickaxe", "axe", "spade")

@DoomRater
Copy link

Have any of these inconsistent results with harvest levels been reported as bugs? I understand that certain blocks really don't have default harvest levels since they can be harvested by hand, but a method that tells us what tool the block should be harvested by should never return null of there's an actual tool used to harvest it!

@lehjr
Copy link
Collaborator

lehjr commented Dec 23, 2015

Hard to say really, but since the focus is on 1.8, 1.7 bugs really won't get any attention now anyway.

@DoomRater
Copy link

Bugs are bugs, but in the interest of getting things working, how is WAILA reporting effective tool on blocks, and can we use a similar method?

@lehjr
Copy link
Collaborator

lehjr commented Dec 23, 2015

is that in WAILA itself or an addon?

@lehjr
Copy link
Collaborator

lehjr commented Dec 23, 2015

@MachineMuse
Copy link
Owner

good call.

@lehjr
Copy link
Collaborator

lehjr commented Dec 24, 2015

It would be nice to be able to get the hardness of the block without the coordinates.

@MachineMuse
Copy link
Owner

The reason for that is probably so that it can check its tile entity or metadata if it has one. Blocks may want to vary their harvest levels, too, just like our tool does :P

@lehjr
Copy link
Collaborator

lehjr commented Dec 25, 2015

Testing a some new code now. Good news is that so far the modules all seem to be working properly. Bad news is having to get the coordinates from inside of canHarvestBlock in order to get the hardness.

I'll hold off on submitting a PR until it can be looked at and maybe a better way suggested.
Edit: deleted that and redid it. Eliminated hardness checking due to issues with getting coordinates. I'll submit a PR once I successfully test it.

@lehjr
Copy link
Collaborator

lehjr commented Dec 28, 2015

There, finally got something that seems to work well for everything except the axe module is too slow with Treecapitator Mod #674

Edit: works great in dev environment, but not in a game. Guess it will be a while longer.
Edit 2: So here was the issue. The tracks in the dev environment return a the pickaxe as the harvest tool while in a normal game, the required tool is a crowbar. So my code does actually work fine. In order for the PowerFist to be able to harvest the tracks, it requires the Omniwrench module to be installed.

MachineMuse added a commit that referenced this issue Dec 29, 2015
Fix for Hard to break certain blocks #628
@VladimirMangos
Copy link

Thank you for hard work at this issue!

@MachineMuse
Copy link
Owner

ok, cool :D going to close this then since it sounds like all the sub-tasks were solved.

@lehjr
Copy link
Collaborator

lehjr commented Jan 28, 2016

I still have to fix an issue with mining the meteors from AE2, but everything else seems to be OK.

@MachineMuse
Copy link
Owner

That's fine, we can't guarantee compatibility with every mod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests