Fix for issue #1217 (dig spell evaporating TE machines)

[DoomFruit]

Sticking a compiled and re-obfuscated version of the 009 dev build into my full modded setup for further testing, I was able to replicate issue #1217 (dig spells evaporate TE machines) on Thermal Expansion machines, ComputerCraft computers and Project Red frame motors - the spell hits and the block just vanishes into thin air, dropping nothing.

Since the actual effect code in Dig.java seems to be a re-implementation of tryHarvestBlock() in the vanilla ItemInWorldManager.java class with some additional checks tacked on for the magic parts, my approach was to trace back what the vanilla function does and start adding subroutine calls into the Dig spell's effect code until things started working again.

I've tested this modified version on TE machines and diamond ore - fortune, silk touch and unmodified spells all behave as would be expected.

[Mithion]

I had this implementation before - it caused certain blocks to drop double due to the multiple function calls around breaking/harvesting. I don't remember which ones but I know they had tile entities.

This will need to be tested on all vanilla/AM2 blocks with a tile entity before it can be merged in, to ensure that issue is not still present.

[DoomFruit]

I've just finished testing vanilla tile entitied blocks in my full modded setup. Unless otherwise stated, I used a projectile dig spell with fortune 3 and interact with non-solid blocks.

-Beacon (unlit/lit): Normal behaviour in both cases (drops 1 beacon)
-Brewing Stand (full/empty): normal behaviour in both cases (drops 1 stand, and the correct contents if present)
-Chest (full/empty): Normal behaviour, drops chest and contents if present. Double chests are handled correctly.
-Command Block: can't spawn one in.
-Comparator (default/configured): normal behaviour.
-Daylight Sensor: normal behaviour.
-Dispenser (empty/full): normal behaviour.
-Dropper (empty/full): normal behaviour.
-Enchantment Table: normal behaviour.
-Ender Chest: 8 obsidian for the fortune dig spell, 1 ender chest for the silk touch spell. Normal behaviour.
-End Portal: dig spells do not break the block, as expected
-Flowerpot (empty/full): normal behaviour (get the pot back, or pot + single flower if full)
-Furnace (empty/full): normal behaviour with items in the input, output and fuel slots.
-Hopper (empty/full): normal behaviour.
-Mob Spawner: I have Ender IO installed and consequently I get the EIO broken spawner block which would result if I used a pickaxe. Normal behaviour.
-Noteblock (default/configured): normal behaviour.
-Piston (retracted/extended): normal behaviour whether I hit the base or the extended head with the dig spell.
-Sign (default/written): normal behaviour
-Skull (various different types): normal behaviour.

Vanilla seems to be fine - no duplication or missing items. I'll take a look at AM2 blocks later on.

[DoomFruit]

Here are the results of testing on AM2 blocks (this is in a pure AM2 dev environment, not my full modded setup). Testing was again done using projectile dig spells with interact non-solid blocks and either fortune 3 or silk touch. Comparison was done against using a non-enchanted diamond pickaxe or axe, whichever was appropriate for the block in question.

I found a few unrelated bugs while testing - some of which were minor and were fixed, others requiring more attention were not fixed here because they fall outside the scope of these changes (making the dig spell work properly). One of them is pretty serious (client crash when breaking an active calefactor... but not if you use a spell to do it)

-Arcane Deconstructor (empty/full): No duplication of dropped items, but the keystone seems to have absolutely no effect on usage or breaking. This seems to be caused by the inventory slots for keystone runes being assigned improperly and has been fixed. Works fine after that.
-Arcane Reconstructor (empty/full): Works fine
-Armour Imbuement Table (empty/full): works fine.
-Astral Barrier: Non-matching keystone caused duplicate blocks to drop. The dig spell didn't have code to handle rune keys, this is now fixed. Runed astral barriers now do nothing if you don't have a matching keystone, and drop normally (barrier + focus, if present) if you do.
-Black Aurem: Drops normally whether a plain pickaxe or a dig spell is used.
-Broken Power Link: These blocks are strange. Placing another block on the broken link (as per the instructions in the Arcane Compendium) does not remove it, even if you leave it for a full minute. You need to remove the source or destination blocks in order for the broken link block to be erased. Dig spells do nothing against the broken power link, neither does a diamond pickaxe. While it is an issue that should be resolved at some point, it's not related to this pull request and so I'm not going to start tinkering with the power system at this time.
-Calefactor: Attempting to use a pickaxe to break one which is currently active will crash the client:

net.minecraft.util.ReportedException: Rendering Block Entity
at net.minecraft.client.renderer.tileentity.TileEntityRendererDispatcher.renderTileEntityAt(SourceFile:107) ~[TileEntityRendererDispatcher.class:?]
at am2.blocks.renderers.TechneBlockRenderHandler.renderWorldBlock(TechneBlockRenderHandler.java:72) ~[TechneBlockRenderHandler.class:?]
(...)
Caused by: java.lang.IllegalStateException: Already tesselating!
at net.minecraft.client.renderer.Tessellator.startDrawing(Tessellator.java:219) ~[Tessellator.class:?]
at net.minecraft.client.renderer.Tessellator.startDrawingQuads(Tessellator.java:212) ~[Tessellator.class:?]
at net.minecraft.client.renderer.RenderBlocks.renderBlockAsItem(RenderBlocks.java:8159) ~[RenderBlocks.class:?]
at net.minecraft.client.renderer.entity.RenderItem.doRender(RenderItem.java:139) ~[RenderItem.class:?]
at am2.blocks.renderers.CalefactorRenderer.RenderItemAtCoords(CalefactorRenderer.java:115) ~[CalefactorRenderer.class:?]
at am2.blocks.renderers.CalefactorRenderer.renderAModelAt(CalefactorRenderer.java:107) ~[CalefactorRenderer.class:?]
at am2.blocks.renderers.CalefactorRenderer.renderTileEntityAt(CalefactorRenderer.java:120) ~[CalefactorRenderer.class:?]
at net.minecraft.client.renderer.tileentity.TileEntityRendererDispatcher.renderTileEntityAt(SourceFile:100) ~[TileEntityRendererDispatcher.class:?]

Interestingly enough, a dig spell works perfectly. Perhaps it's something to do with the block break animation? This exact same error has already been found in issue #1207, though the given steps to replicate it are incorrect - I can interact with a running calefactor, I just can't use a pickaxe on it. While crashing like this is a definite problem, it's outside the scope of this pull request and needs separate attention.

-Warding Candle: breaks as normal.
-Celestial Prism: pickaxe and both types of dig spell return the prism block, ready to be placed again. Works fine.
-Crafting Altar (alone, or part of the multiblock): works fine.
-Crystal Marker: works fine.
-Essence Conduit: works fine.
-Essence Refiner (empty/full): Works fine, drops everything present inside the refiner and nothing else. One thing I noticed while testing is that re-placing the refiner back in the spot where it was taken from seems to retain the power link, even after a minute of no block being present there. Behaves as expected when key runes are installed.
-Everstone: nothing happens, but everstone is hard-coded as one of the disallowed blocks and so I'm assuming that this is intended behaviour.
-Flicker Habitat: works fine
-Flicker Lure: works fine
-Spell Rune: erased in one hit, no drops - intended behaviour?
-Inert Spawner: breaks normally, drops phylactery if present.
-Inscription Table: works fine
-Keystone Chest: breaking it with runes present (using axes or dig spells) caused it to destroy all items in its internal inventory and drop the chest, regardless of whether you had the right combination in a keystone or not. This was due to super.breakBlock() not being included in the KeystoneUtilities check. I've put that in the loop and it now no longer breaks if you don't have the right keystone, but the items inside are still destroyed if you do (dropping only the chest). This is the result of some extremely weird stuff in Block.java where breakBlock() is defined, but has no function calls anywhere in the entire vanilla or Forge source tree. It's obviously getting called somewhere, since items get dropped from chests when you break them, but I do not know how.

My workaround was to hook the onBlockHarvested function in BlockKeystoneChest.java and explicitly zero out the runes if the breaking player has access. This resets it to the default behaviour and it drops the chest along with its contents if you have the correct keystone. If you don't, diamond axes and dig spells just bounce right off it.

This was not directly related to the dig spell, but the fix was short, it was an obvious bug and I therefore may as well add it in to the pull request.

-Keystone Door: Works fine.
-Keystone Receptacle (gateway controller): Get an un-localised chat message when trying to break a keyed receptacle, fixed that and now it works fine (assuming that the intended behaviour is as the tooltip says - clear the key, then break it
-Keystone Trapdoor: works fine.
-Lectern (empty/full): works fine
-Magician's Workbench: same issue as the keystone chest (doesn't drop inventory when broken while rune-keyed), I gave it the same fix. Additionally, I noticed that crafting upgrades aren't returned when you break the workbench. I've changed it so that upgraded workbenches now drop the crafting upgrade as an item when broken - this seemed like sensible behaviour.
-Mana Battery (empty/full): works fine, batteries keep their power when broken with a pick or dig spells.
-Obelisk (with/without fuel items): Works fine.
-Occulus: works fine
-Otherworldly Aura: destroyed without any drops when using a pickaxe or dig spell, seems to be fine.
-Particle Emitter: Works fine. Dig spells with non-solid block interactions are able to break invisible particle spawners where a pickaxe will completely ignore it - intended behaviour?
-Seer Stone: works fine.
-Slipstream Generator: works fine.
-Spell-Sealed Door: I'm not entirely sure on what this thing does, since there's nothing in the arcane compendium about it. It seems unfinished, so I'm reluctant to touch it any further. Like the keystone door, it appears to drop a normal wood door. I don't seem to get around the damage values to make it drop a spell-sealed door - I can only get keystone doors out of it. It also can only be broken using an axe - spells don't do anything (this appears by design). Additionally, it seems to leave phantom upper doors behind when you break one. Since I can't find an easy fix for the first issue, and since it's not directly related to dig spells (it is, in fact, immune to magic), I'm not going to look at it any more in this pull request.
-Summoner: Seems to work fine.

[Mithion]

The power link blocks are in fact destroyed when replaced with a block, however the moment there is not a block there anymore it will be re-created (as the power system is still trying to find a link through that point and failing). This is the intended behaviour. The block is meant to be an information block that appears when there's a broken link in the power system (ie a creeper blows out a conduit) so you can find out where it was and rebuild it. At the same time, they're designed to not hinder building in any way (especially when invisible).

The Spell-Sealed door I designed during a livestream, though it appears some of the code was also lost in the same event that caused my other problems. Basically it's a door where you can put a spell in the UI, and casting that same spell on the door is the only way to open it. Without a spell in the "lock", it should function like a normal door. At least that was the goal, anyway.

The Particle Emitter shouldn't be targeted by the dig spell if it's invisible - that's a bug, likely in the raytrace.

[Mithion]

Amendment:
The first two points were more for the sake of clarity and don't make sense with this PR, but in the interest of making dig work, I think the particle emitter bug should be taken care of.

[DoomFruit]

OK. Should an invisible particle emitter be completely unbreakable by players (through whatever means, be it AoE dig spells/Tinker hammers/quarries/whatever) or just immune to spells?

If it's the former, I'll put a general "don't break this block if invisible" check in the main block class and if the latter, I'll put a special case check in the dig spell.

[Mithion]

I would say if invisible == can't break, period.

[DoomFruit]

Done. Invisible particle emitters can no longer be broken by players, though I noticed 2 more things while testing it: 1) the invisible emitter will still stop spell projectiles which have the "interact with non-solid blocks" modifier, 2) you can still right-click on invisible emitters (empty-handed) if you know where they are to bring up their GUI. Intended, or not?

[Mithion]

Neither of those are intended - with the exception of actually spawning the particles, and preventing other blocks from being placed there, no interaction with the particle emitter should be possible while it's invisible.

[DoomFruit]

So far, I've managed to stop projectiles and explosions from doing anything to invisible emitters, but preventing right-click interactions is pretty hard. What happens is that when you right-click on the invisible emitter, it turns visible and stops emitting particles. At that point, you can "break" it with a single hit - but this is client-side only and the dead emitter will return if you try to place another block over the top. You are also able to open the GUI at that point and make changes.

Removing everything in onBlockActivated and just telling it to return false still causes the particle emitter to re-appear client-side and stop emitting particles, even though nothing should be happening. It seems to be some kind of client-server desync issue, since if you exit to the main menu and reload the world, the particles return and the emitter turns invisible again. I cannot figure out what is causing this.

Any ideas?

[Mithion]

Might be something to do with the tile entity - check the Update() function or see if SetShow() is being called when it shouldn't.

[DoomFruit]

No luck there either. Whatever kind of server-client desync is going on, it happens when you right-click on an adjacent block to the invisible emitter.

I do have a solution of sorts - by hooking PlayerInteractEvent, I'm now blocking every right or left-click on a block if there's an invisible particle emitter attached to the facing of that block. Since it's a client-server desync issue, only hooking that event on the client-side seems to be functional (thus reducing server overhead).

To make it easier on users, I've also moved the functionality which saved and loaded emitter settings to the crystal wrench's alternate mode, so you don't have to keep taking out your crystal wrench to reveal invisible emitters and then putting it away to modify them.

One flaw with my solution here is that you are no longer able to use machines or break blocks directly (magic and AoE tools are still effective) which are hidden behind an invisible particle emitter.

Finally, I noticed another issue whilst testing this. Invisible particle emitters aren't invisible if you load in right next to them. You can see their outline, but you can't interact with them. They turn invisible if you cause a block update next to them, pull out a crystal wrench and then put it away, or change the settings on another emitter nearby.

In conclusion: it works, but it's not pretty.

[DoomFruit]

And since I'm not entirely sure how to synchronise files I haven't changed with the master fork, this pull request might try to eat the changes you made to EntitySpellProjectile. I've manually stuck the current version of EntitySpellProjectile.java into my fork and branch, but it still thinks that something's changed. Keep an eye out if you do merge it...

[DoomFruit]

Ignore the previous post, everything's now synchronised.

[DoomFruit]

Notifications for non-matching keystones have been added to this branch as well.