XCompWiz / Mystcraft-Issues

Issue tracked and localization repo for the MInecraft mod Mystcraft
xcompwiz.com
31 stars 23 forks source link

Symbol Portfolio NBT can grow excessively large #171

Open EyeDeck opened 7 years ago

EyeDeck commented 7 years ago

Short backstory, I was hunting for a Chromaticraft "Dense Generation" page for quite some time, and Reika set the drop chance on that page very, very low. Lots of exploration, a villager spawner (feeding into a TiCon smeltery) and several hundred sealed notebooks later, I was disconnected with an error typical of an item with an NBT tag waaay too big, and attempting to reconnect gives me similar errors, e.g;

I checked the player save file with the portfolio that disconnected me, and found this: untitled3 That's one entry for every individual page, not just page type. Unsurprisingly, 15,000 of these entries exceeded the 1MB-per-item limit that the game engine imposes.

I assume it'll take some reworking of the interface to accomplish this, but the immediate solution that comes to mind is to rewrite it more like a chest, e.g. store 127 pages per type per NBT entry (the count tag that already exists is a byte), and only after that's full make a new one. The interface already tallies page counts greater than 1 correctly, though withdrawing stops working when it gets to a non-1-count page, and obviously insertion currently splits stacks into individual tags.

Also, here's the player save that was giving me trouble: 51742687-d54a-431b-b54e-8b10505afc52.zip I solved the problem by loading the save in NBTExplorer, and splitting half of the pages in the portfolio off into another empty portfolio that had already been in my inventory, then reloading the save.

XCompWiz commented 7 years ago

Well there's a fun border case. :D

I might implement your suggestion, but it isn't a perfect fix. It just defers the problem (potentially enough that it wouldn't matter, but the border case would remain).

The catch for your solution is I would need to merge in any added pages, which at even 1k entries could take a moment computationally.

It's also worth noting that page quality is coming; that will both make this issue easier to hit and make the proposed solution less effective.

I'll have to think about clever ways to solve this. No matter what I might need to apply a hard limit to the number of items that can be stored in the portfolio as an actual "fix."

KlutzLament commented 7 years ago

A recent pull in Forge gives a potential solution for 1.10.2: https://github.com/MinecraftForge/MinecraftForge/pull/3342

Not much help in 1.7, though...

EyeDeck commented 7 years ago

If it were applied to the current release version of Mystcraft, according to some back-of-the-envelope math, 127 pages per tag would increase total potential page storage per portfolio to somewhere in the range of 1-2 million. For what it's worth, in the example that prompted this issue report, the average data length per page was about 59 bytes.

With page qualities being added, depending on how arbitrary their qualities are (say, boolean good or bad, or multiple randomly generated floats determining separate page statistics), that could put us to a theoretically worse situation than current, not only making pages not stack, but adding additional data to be stored, increasing NBT length per-page.

I can't say I'd blame you if the ultimate solution ends up being to just limit page count.

XCompWiz commented 7 years ago

We'll see what I can do. Limiting duplicates in the portfolio is a good idea either way. :) Good note on how large the difference is. Is that in this instance? Can you attempt to determine how effective it would be here?

EyeDeck commented 7 years ago

In this instance, with all of the mods installed, there were about 385 different pages. Of those 15,000 pages, only 54 of them would require more than 1 tag (2 would take 3 tags) if fully utilizing a byte for count: image So in this particular instance 385 tags + 56 duplicate tags is 441, and at an average length of 60 bytes, this would reduce the overall data length from ~800KB to ~26KB. Here we're only looking at a factor-of-30 or so decrease, but even if one were to triple the page count to 45,000, since all but a small percentage of pages would still fit within their original tag, total size would only increase slightly (~33KB). As noted before, it would be somewhere over 1 million semi-randomly assorted pages (as would drop from sealed notebooks) before NBT size became an issue again.

Judging by the automatically generated symbols.cfg file in this instance, there had been, at one point, BoP and MFR installed, which, along with IC2, Metallurgy 4, Forestry, Witchery, and a few other mods that add 1-3 fluids, brings a closer-to-worst-case scenario up to 509+56 tags, and the maximum byte count I noticed for any page was about 90 (I think it was modmat_tile.projectred.compatibility.liqcondredmetal). Therefore, in a close-to-worst-case scenario, at 565 * 90 bytes, we're only up to about 50KB for a 15,000 page symbol portfolio Still not bad.

XCompWiz commented 7 years ago

Thanks! Good stats. I very much appreciate your work on this. :)

DwellerBenthos commented 5 years ago

Is there any fix on this yet? I apparently put too many pages in a portfolio in a writing desk and get kicked off the server. Now every time I try to even enter the area with the writing desk, I get the same crash, and not even trying to open the desk interface.

https://imgur.com/a/pE0vr1T