Fix bounds error when loading negative block IDs

[mushroomhostage]

Fixes the error when pasting schematics with large block IDs which which wrap around and cause this out of bounds error:

19:01:33 [SEVERE] java.lang.ArrayIndexOutOfBoundsException: -28
19:01:33 [SEVERE]   at org.bukkit.Material.getMaterial(Material.java:438)
19:01:33 [SEVERE]   at com.sk89q.worldedit.bukkit.BukkitWorld.isValidBlockType(BukkitWorld.java:844)
19:01:33 [SEVERE]   at com.sk89q.worldedit.EditSession.rawSetBlock(EditSession.java:178)
19:01:33 [SEVERE]   at com.sk89q.worldedit.EditSession.flushQueue(EditSession.java:731)
19:01:33 [SEVERE]   at com.sk89q.worldedit.WorldEdit.handleCommand(WorldEdit.java:1265)
19:01:33 [SEVERE]   at com.sk89q.worldedit.bukkit.WorldEditPlugin.onCommand(WorldEditPlugin.java:204)
19:01:33 [SEVERE]   at com.sk89q.bukkit.util.DynamicPluginCommand.execute(DynamicPluginCommand.java:44)
19:01:33 [SEVERE]   at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:166)
19:01:33 [SEVERE]   at org.bukkit.craftbukkit.CraftServer.dispatchCommand(CraftServer.java:469)
19:01:33 [SEVERE]   at net.minecraft.server.NetServerHandler.handleCommand(NetServerHandler.java:914)
19:01:33 [SEVERE]   at net.minecraft.server.NetServerHandler.chat(NetServerHandler.java:874)
19:01:33 [SEVERE]   at net.minecraft.server.NetServerHandler.a(NetServerHandler.java:857)
19:01:33 [SEVERE]   at net.minecraft.server.Packet3Chat.handle(Packet3Chat.java:33)
19:01:33 [SEVERE]   at net.minecraft.server.NetworkManager.b(NetworkManager.java:234)
19:01:33 [SEVERE]   at net.minecraft.server.NetServerHandler.a(NetServerHandler.java:118)
19:01:33 [SEVERE]   at net.minecraft.server.NetworkListenThread.a(NetworkListenThread.java:78)
19:01:33 [SEVERE]   at net.minecraft.server.MinecraftServer.w(MinecraftServer.java:557)
19:01:33 [SEVERE]   at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:455)
19:01:33 [SEVERE]   at net.minecraft.server.ThreadServerApplication.run(SourceFile:490)

To reproduce:

1. Install MCPC 1.2.3 with IndustrialCraft^2 from http://www.mcportcentral.co.za/index.php?topic=1692.0
2. Load an MCEdit schematic containing machine block IDs , example: http://dl.dropbox.com/u/65303659/Schematics.zip
3. //schem load XPHCFactory
4. //paste

Without the patch, the above error occurs, and the IC2 machines and cables are missing. With the patch, they're pasted successfully.

[hqSparx]

please include this into main we project - its working great!

[Erikvv]

This issue is solved for //copy and then //paste but still occurs when saving and loading a schematic and then pasting.

[avogatro]

This patch is crucial for any mods, that add additional blocks to vanilla.
Especially for the Tekkit Project.
I really think it should be implented as an option.

[Sleaker]

Honestly, this should not be pulled into the main WG plugin. You should maintain your own compatible version for Tekkit instead of expecting WG to maintain compatibility for you.

[Erikvv]

Minecraft 1.3 has added blocks with block id's above 127 (133 I think at this point). So this bug will have to be fixed in order for WorldEdit to work with Minecraft 1.3.

@Sleaker: I disagree. Anyone who works on this mod (WorldEdit) obviously understands that modding is an important part of minecraft and bukkit. This 'bug' affects any other mod which adds blocks, not just tekkit.

Ofcourse it is only a matter of time before 256 block id's are insufficient. In fact some servers have already gone over this limit. Therefor there are bukkit builds which allow block id's up to 4096 (so-called "4096 Block ID Fix", Forge API already supports it). Ideally WorldEdit should support block ID's up to 4096.

[Sleaker]

WorldEdit is currently built specifically for Bukkit. Until it's not specifically built for Bukkit this pull request should not be incorporated into the core plugin. In addition, the 'fix' is dealing with Invalid Block IDs to begin with, ones with negative values. I don't see how allowing negative block ID values to begin with is necessary for this plugin to even touch. If/When WorldEdit is built to work with a system that supports higher blockID values negative IDs wont even be used. Which again, means that this pull is completely unnecessary.

The issue is that a schematic is being built with invalid block IDs, not that worldedit can't import them.

[Erikvv]

They arent negative, they are just bigger than 127 (since Mojang now does this too this is totally valid). I'm guessing WorldEdit thinks they are negative because it interprets them as an 8-bit signed integer while in fact it is an unsigned integer.

[Sleaker]

ahh gotcha, the whole +256 thing is what's throwing me off. Changing the stored types is fine, but worldedit shouldn't include fixes to wrongly saved schematic files. It should just be adjusted to account for blockIDs above 128.

[hqSparx]

http://mcportcentral.co.za/wiki/index.php?title=Ports_for_1.2.5

[Sleaker]

So bukkit does or doesn't support full 4096 block IDs?

[avogatro]

well thx sleaker. I think I have to wait until forge 3.3.8 get into tekkit. because stable tekkit is a bit outdated (forge 3.1.3)
Can't kick people out of the server right now for upgrading forge, would cause panic :D

I did not see any commit of bukkit with 4096ID patch. Maybe it is not in the master branch yet.

Personally I think forge + bukkit is the next step to go, not forge xor bukkit.

[Sleaker]

in reference to Forge supporting it. Bukkit doesn't support non-MC API, I don't see how this is related at all. If MC doesn't support block IDs above 128 yet, then it shouldn't be changed at all.

As far as the unsigned int comment, Java doesn't have unsigned integers so this isn't an issue at all.

[zml2008]

I've heard of this issue before, but the fix in this pull request is not the correct fix. Bad values are being passed into BaseBlock, so wherever the bad values are coming from should be fixed, not BaseBlock.

[mushroomhostage]

If anyone wants a WorldEdit with this change, I've uploaded a build with it here: EDIT: No unofficial builds . Note it still may not properly handle all high block IDs in all situations, but it at least works for me when loading and pasting schematics.

[hqSparx]

would be also nice to have proper metadata handling, most of RP blocks are being skipped

[mushroomhostage]

I hear someone is working on 4096 compatibility for WorldEdit (forgot where I heard it, #minecraftforge or #sk89q? sorry I don't have more information), it may be a better, more comprehensive and correct change to take than this patch.

@hqSparx the RedPower problem may be unrelated; I was able to paste IndustrialCraft^2 machines, IIRC (not completely sure, its been several months since I tried).. if the blocks are tubes or microblocks, could be an issue with tile entities not being copied.

[md-5]

This is still a completely valid issue, none the less.

[mushroomhostage]

Meanwhile, I'm maintaining this change in my fork on GitHub in case anyone wants it. Cheers.

[zml2008]

@md-5 There is no question that this is a valid issue. Spout is not limited to bytes for block id storage, and would eventually suffer from this issue too, as well as singleplayer WE with mods. However, this pull request is not the correct solution.

[md-5]

So lets design a proper intermediate standard and hope we don't land up with more competing standards.

[mushroomhostage]

@zml2008 would using an 'int' for the block IDs be a better solution?

[md-5]

I think short is the answer, not int.

[Sleaker]

@mushroomhostage - he was referring to the +256 hack for mis-saved data. The issue is in other locations that cast to byte. So the solution needs to go find where this is being done, and fix it at those steps.

[md-5]

This has been fixed in a commit yesterday