skyboy / MineFactoryReloaded

104 stars 75 forks source link

Prevent ArrayIndexOutOfBoundsException #548

Closed andrewminer closed 7 years ago

andrewminer commented 8 years ago

fixes #547

skyboy commented 8 years ago

This has been a long standing issue as a result of other mods (wish I knew which ones) randomly altering the metadata of blocks in the world (maybe leaves in particular? doesn't much matter) that I'm loath to account for because it means if I ever add additional rubber leaf types (like mega leaves or sacred leaves) then these blocks will randomly appear on regular trees. That's not something that I want happening.

andrewminer commented 8 years ago

I can totally understand not wanting to clear up after someone else's problem. The difficulty in the meantime is that I (and probably other players) have a completely corrupted world. Whenever I get close enough to that tree (wherever it is: I still haven't found it), my game crashes. I'd personally much rather have the possibility of some funny-looking leaves on a tree than a trashed world which I might have put hours and hour of work into.

As for the source of the issue... could it have something to do with leaves drying out? I encountered this while exploring a BoP bamboo forest biome which borders a lot of much dryer biomes. Perhaps the affected tree is being altered by one of those biomes and/or the leaves lose the right meta data when they dry out.

skyboy commented 8 years ago

You can, in the meantime, clear those leaves by getting near the point you crash and running /cofh clearblocks with a larger diameter than your view distance for rubber leaves; or if you want only the affected metadatas: 2, 3, 10, 11, 6, 7, 14, 15 (the latter four being player-placed metadatas).

Additionally, this build should create an "invalid set block attempt" crash any time anything creates an invalid metadata; though it will not trigger for existing blocks

andrewminer commented 8 years ago

So, you'd like me to run your special build to see if I can track down what's causing the blocks to become corrupted? Sure. I can give that a try.

skyboy commented 8 years ago

Nothing triggered the crash since?

andrewminer commented 8 years ago

Sorry! I've been working on other projects lately and I haven't tried to reproduce the issue. I'll see if I can reproduce it over the weekend.

andrewminer commented 8 years ago

I've managed to reproduce the the crash, but I'm afraid the stacktrace doesn't seem to say very much. Here's a gist of the full log file.

skyboy commented 8 years ago

Hopefully that corrects the source of the issue. I don't know why or where the deeper source is coming from (passed metadata value of 0 being modified to a 2), so I can't attempt to address that, but I can at least attempt to squash the symptoms.

andrewminer commented 8 years ago

I can't say I understand your fix, but I'd be happy to try it out. Do you have another build for me to check out? Will the fix work on existing worlds, or will I need to start over again?

skyboy commented 8 years ago

A build should be live on curse as an alpha by now; It should be fine in existing worlds, but I can never really promise that with alphas lest people try to include them in mod packs. (and if it isn't, I do need to know, just not from ten thousand people)

As far as the reasoning behind the fix: Your stacktrace shows that MFR's own tree generator is creating the invalid leaves, which if you look at the diff should be impossible because it only uses and has only ever used metadata 0 for placing leaf blocks. So the only two things that make sense to me are: A) another mod is modifying leaves either in the superclass method I was calling (setBlockAndNotifyAdequately) via ASM addressed by overriding the method with my own implementation; or B) another mod was modifying leaves in the setBlock chain of methods, which may be fixed by the extra call to setBlockMeatdata, assuming whatever mod is doing the editing doesn't also do the same change in this method. The possibility this is caused by another mod blindly editing other mod's data is a bit of a problem, that never does anything but cause problems (see: people "repairing" TE cells/tanks/etc. into creative versions because of bad implementation of a repair mechanic in another mod). There's the off-chance this isn't a mod using ASM, and is actually still a bug in vanilla's setBlock code, but fuck me if I need to try digging in that mess of a method again to understand what's wrong this time.

andrewminer commented 8 years ago

Heh. I'll give it a shot, then. I'm a professional software engineer, so you'd figure I'd be all over combining my interest in Minecraft with my love of my work. But, the truth is, after all day trying to figure out other people's code at work, I just can't bring myself to dive into modding for exactly those reasons. Kudos to you and everyone else who puts up with all this crap to make such great additions to the game. 👏

andrewminer commented 8 years ago

I've upgraded to your newer version (2.8.2-B1-192), and I'm still seeing the problem. :-(

https://gist.github.com/andrewminer/f11d58a580099b5d2f5ce93e3d337d03

skyboy commented 7 years ago

Ah to hell with fixing the source of the issue. It's probably some other mod being poorly written. Whatever, people with that mod can just deal with any issues that may crop up from additional rubber leaf types.