Hash collision when loading a map caused by custom ThingDef

Started by Jaxxa, August 30, 2016, 06:09:17 PM

Previous topic - Next topic

Jaxxa

Hash collision when loading a map caused by custom ThingDef.

This error is Similar to https://ludeon.com/forums/index.php?topic=25171.0 and https://ludeon.com/forums/index.php?topic=25076.0 but the solutions presented there are not working for me. Kilroy232 was able to work around it by just removing the assembly and Hatti fixed by setting a defName (I am assuming that he was not using a Custom C# ThingDefs), so that was a different root issuing causing the same errors.

In Alpha 15 I have been unable to successfully load any map while I have a .dll loaded that contains a class that inherits from "Verse.ThingDef". I am not entirely certain if there is something additional that needs to be done in this latest Alpha or if there is a new bug/change in the Core game that means we are unable to do this.

From the looks of the error log everything is being given a short hash of 0.
I have tried stripping everything out of one of my mods in order to identify what was causing this issue.
I have managed to consistently reproduce the error using a .dll with only a singles .cs file and no .xml files (Except About.xml)


public class ShieldBuildingThingDef : Verse.ThingDef
{
}


I am not the only one having this issue, I know rikiki and haplo have mentioned it (or very similar) on slack.

I have attached the error log and mod I was using to reproduce this error.

I have been looking through ILspy but I have not helped me fix the issue.
Any help would be appreciated.

Edit: Maybe this would be better done using Comps.

[attachment deleted by admin - too old]

Rikiki

Problem confirmed. Having a class inheriting from ThingDef leads to "Short hash error" when loading a save game.
Note: it was working fine on A14.

1000101

Can anyone produce minimal code which positively reproduced this error?  Both C# and xml?
(2*b)||!(2*b) - That is the question.
There are 10 kinds of people in this world - those that understand binary and those that don't.

Powered By

Jaxxa

Quote from: 1000101 on August 31, 2016, 02:16:21 AM
Can anyone produce minimal code which positively reproduced this error?  Both C# and xml?

That should be in the .zip file attached to the first post.
Let me know if you wanted something else, or I can put it on GitHub later tonight.

But as we talked about on Slack using comps is probably better if I can get that working for everything.

RawCode

provide complete mod, source not required as long as no obfuscation is used, both C# and XML parts.

l2evy

I too have a custom ThingDef and suffer the same fate.

I really hope this is a bug and not a core change. The person that did the DLL for the mod I am caretaker of is long gone to help me, and C# I am the worst at heh. I am learning as I go though but I may need to poke ya Jaxxa sometime about what you found out on the process of switchin to comps (which you seem highly skilled in C# so you likely know already how to do it). I will read into comps tomorrow though, I was sleeping 2h ago I think..or something #TheStruggleIsReal

Here is my full decompiled DLL though if you/anyone need to look at it for reference with this issue.
https://github.com/l2evy/storage-crates-dll

Also the complete mod itself that it comes from just incase
https://github.com/l2evy/Storage-Crates

Here is to hoping for a 15c fix soon if its not a core change.
Mod Maintainer/Coder of:

skullywag

I did some debugging last night on this with haplo on the slack chat, the only thing we could see that would cause this from a pure code logic pov was if the list of defs in the def db didnt have defNames set, this would make all shorthashes stay as the default 0 (I think) but nothing in the logic suggests that could even happen. So its porbably something further up the chain, theres some reflection used in the "set all hashes" method that calls the individual set hash method that im unfamiliar with so it could be there....maybe...
Skullywag modded to death.
I'd never met an iterator I liked....until Zhentar saved me.
Why Unity5, WHY do you forsake me?

1000101

I'm wondering about your example having the def class in the root namespace and what happens if it's put into a proper namespace and properly referenced.
(2*b)||!(2*b) - That is the question.
There are 10 kinds of people in this world - those that understand binary and those that don't.

Powered By

Fluffy (l2032)

I think I've found the issue (copy/pasta from slack, ignore the awkward formatting).

fluffy [12:18] 
ok, Tynan adds two extension methods to `Type`

skullywag [12:18] 
k

fluffy [12:18] 
`AllSubClasses` and `AllLeafSubClasses`

[12:18] 
`AllSubClasses` iterates types to check if they are inherited from a given base type, thats all fine and dandy

[12:19] 
`AllLeafSubClasses` takes the list from `AllSubClasses` and then filters out *all* types _that have *any* child class_

[12:19] 
meaning, if you extend ThingDef, ThingDef now has a childclass

[12:19] 
and will be filtered out of `AllLeafSubClasses`

Fluffy (l2032)

#9
the solution should be simple, in Verse.ShortHashGiver, the top lines should be change from


public static void GiveAllShortHashes()
{
ShortHashGiver.takenHashesPerDeftype.Clear();
List<Def> list = new List<Def>();
foreach (Type current in typeof(Def).AllLeafSubclasses())


to


public static void GiveAllShortHashes()
{
ShortHashGiver.takenHashesPerDeftype.Clear();
List<Def> list = new List<Def>();
foreach (Type current in typeof(Def).AllSubclasses())


That would create hashtables for abstract types, but they'd be empty anyway.

Master Bucketsmith

Quote from: Fluffy (l2032) on August 31, 2016, 06:23:55 AM
the solution should be simple, in Verse.ShortHashGiver, the top lines should be change from


public static void GiveAllShortHashes()
{
ShortHashGiver.takenHashesPerDeftype.Clear();
List<Def> list = new List<Def>();
foreach (Type current in typeof(Def).AllLeafSubclasses())


to


public static void GiveAllShortHashes()
{
ShortHashGiver.takenHashesPerDeftype.Clear();
List<Def> list = new List<Def>();
foreach (Type current in typeof(Def).AllLeafSubclasses())


That would create hashtables for abstract types, but they'd be empty anyway.
Uh, am I missing something? Aren't those two snippets exactly the same? :P

skullywag

Ive PM'ed this directly to Tynan as well, I felt it warranted him seeing this ASAP.

Edit the 2nd one should be -leaf, fluffy will update shortly.
Skullywag modded to death.
I'd never met an iterator I liked....until Zhentar saved me.
Why Unity5, WHY do you forsake me?

Fluffy (l2032)

ehm yeah, I copy pastad because they are almost the same, then forgot to change it. Basically AllLeafSubClasses should be recplaced with AllSubClasses, assuming that doesn't throw errors further down the line (I doubt it - but RW is a complex piece of code).

Master Bucketsmith

Quote from: Fluffy (l2032) on August 31, 2016, 06:33:49 AM
ehm yeah, I copy pastad because they are almost the same, then forgot to change it. Basically AllLeafSubClasses should be recplaced with AllSubClasses, assuming that doesn't throw errors further down the line (I doubt it - but RW is a complex piece of code).
Thanks, gotcha.

skullywag

If Tynan can let us know if thisll be fixed in 15c soonish we can leave it out of the 1st CCL release for A15, if not we can detour it in CCL I guess until it is. Depends on the timeframes.
Skullywag modded to death.
I'd never met an iterator I liked....until Zhentar saved me.
Why Unity5, WHY do you forsake me?