Hello,

today I scrolled in the Better Questing menu for the Icon selection and I got a crash.
-> https://paste.dimdev.org/emolulovoh.mccrash

I ask under a BQ crash, if this is similar to the crash in the topic, but no.
The BQ Dev. "Funwayguy" can't rly do something to resolve this crash and so he told me, that I should report this crash to you. :=)
He also said, if he can help somehow, you/we can tag him.

Funwayguy last comment in case of my crash question:
"Considering I can't do a lot from my side (besides outright ignoring invalid items which would likely crash something else eventually anyway) it would probably be a better idea to send this to Immersive Railroading's GitHub tracker. If necessary you can tag me in it so I can add additional information if needed."

MC 1.12.2
Forge 2765
IE 85
IR 1.3.3
Track API 1.1

Yeah that seems to be another problem with item deserialization. Basically, all IR rolling stock pieces are the same item, so when the Minecraft ItemStack methods strip the NBT data, they all become the same thing and that causes all sorts of issues.

It's not a de-serialisation issue. IR is forcing the stack size to zero when BQ is mid way through searching its name and tooltips.

Relevant line:
https://github.com/cam72cam/ImmersiveRailroading/blob/master/src/main/java/cam72cam/immersiverailroading/items/nbt/ItemDefinition.java#L20

Hmm, we only do that when the stack is missing required data. How are you deserializing the item?

This makes also a JEI Plugin impossible.

Same way the creative search tab obtains item stacks, or if the item is hidden from that tab, asking the item itself for its default instance.

Search Tab:
https://github.com/Funwayguy/BetterQuesting/blob/1.12/src/main/java/betterquesting/api2/client/gui/panels/lists/CanvasItemDatabase.java#L98

Default Stack:
https://github.com/Funwayguy/BetterQuesting/blob/1.12/src/main/java/betterquesting/api2/client/gui/panels/lists/CanvasItemDatabase.java#L113

Regardless of whether an item is missing data, forcing it to zero is likely to break other things down the line, not just in Better Questing. Machines in particular would likely not take well to having an stack unexpectedly self destruct (using ID for filtering purposes for example) even if, like BQ, they already checked it was originally valid before hand and the subsequent code is reliant on that fact. Even simple GUIs would have issues with tooltips killing the item without the inventory container knowing causing a desync. Sure this would only happen if the NBT is missing but by the sounds of things, you're already have cases where that can and does happen.

...

No. No it isn't and I just explained why. Any other mod or piece of code that instantiates a stack via the item will have the same issue. Stop setting the stack to zero. If your item is missing data then find a better way of dealing with that instead of nuking it and breaking other mods.

How do you recommend I should handle the situation? An item without that metadata that's being used is invalid and has not been correctly instantiated. There is no sane "default" value for that property.

Can you not add pre-checks when used and if the tag is missing/invalid simply not allow it? Same way you'd check isEmpty() really. A user shouldn't be able to use an invalid item anyway.

Also your assumption of "correctly instantiated" pretty much comes down to "only via your mod" because almost nobody is going to know this edge case and even less will code a mod specific exception for you.

Hmm, I'll take a look at implementing getDefaultInstance. I suspect that's the root issue here

I should have the pre-checks already. I can remove the set stack to 0 code.

I am not trying to be difficult with you at all, you don't need to get pissed off at me. I am not asking for you to add an exception for my mod, simply trying to understand what I am doing wrong so I can fix it the right way.

I am also going to change getDefaultItem to return an empty stack. Does that seem like it would break anything to you? Would you rather it return the first item in the creative tab list?

As far as getting correctly instantiated versions of my items, I implement getSubItems correctly. What I suspect is happening is that I never knew that the getDefaultItem function existed and was returning a bad value there which caused your code to barf when I set the stacksize to zero when you tried to use it. Am I misunderstanding something here?

I'm not angry or pissed off, just a bit frustrated trying to explain it after I pointed out the exact problem line in my first message. Sorry if I came across like that.

if you made getDefaultItem() return an empty stack it would just crash again, only earlier. BQ allows users to add/remove tags for quest filters anyway.

Here's how I suggest going about this:

1. Remove the line that zeros out the stack.
2. Using a broken item, test and adjust parts of your mod to ensure checks are being correctly made.
3. Fix getSubItems() and getCreativeTabs() to work on the search tab (I only call for the default stack if I got nothing off the search tab).

Don't worry about getDefaultStack() too much. By default it calls new ItemStack() which most people would be doing anyway. As long as 1 & 2 are fine there's no critical need to override it unless you just want to be extra safe.

Ah ok, thanks for explaining.

3 surprises me and suggests that might be where the root of my issues/confusion lie. I thought that my getSubItems and getCreativeTabs was implemented correctly. I'll take some time and make sure they are functioning as expected and patch them if not (along with removing the set stack size (0)).

I might have read a bit too much into your attitude, IRL work has been nuts, sorry about that.

I'll try to get to this within the next week.