[A15.1284] Hash collision? Mod-added terrain replaces gold ore in vanilla save

[Telkir]

Skully suggested this might be something Tynan would want to look at. There's some vaguely bug-like weirdness going on where a terrain added by my MoreFloors mod is replacing gold ore in a vanilla save for no obvious reason.

Original full details of the problem and how it can be reproduced and fixed are in this thread.

In summary: Gold ore in a vanilla RimWorld save is replaced by one specific terrain added by my mod, MoreFloors, when the save is loaded with that mod active. No errors are being flagged in the console. Changing the defname of the culprit terrain by just a single character fixes the problem.

The def in question is "FloorStoneChequerMarbleSandstone" on lines 361-370 of this file:
http://pastebin.com/Xa40L7rH

Apparently there were known hash collision problems in 0.15.1280 that got fixed - perhaps this is somehow related?

[EldVarg]

I get the same errors too - have 100 mods installed.

(Filename: C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineDebugBindings.gen.cpp Line: 37)

Hash collision between TurretWeaponBaseMannable and  Bullet_PBomb: both have short hash 7180
Hash collision between RPP_RechargeStation_Kitchen_Interm and  Plantsmushroom: both have short hash 61243
Hash collision between RPP_Bot_Kitchen_Adv and  TrapIEDIncendiary: both have short hash 48416

When I changed "RPP_RechargeStation_Kitchen_Interm" to "RPP_RechargeStation_Kitchen_Interm2" the error was removed.


Hashtable should probably not be used on id's - as if you have bad luck even having only 2 id's can collide in the same hash slot.

Edit:
You can use a hashtable that chooses next slot if the one it finds are taken - I guess he does not use that version.
Still it can be filled up - and becomes slow after some amount of full.

[Zhentar]

There is hash collision handling code, but there's an issue with how it works when a mod subclasses a concrete *Def class. Short hashes are assigned based on concrete class; MySpecialSnowflakeThingDef : ThingDef things are a member of two different *Def concrete classes and will go through short hash assignment in both the MySpecialSnowflakeThingDef collection and the ThingDef collection. If MySpecialSnowflakeThingDef goes through short hash assignment before ThingDef, then the ThingDef short hash assignment won't check against whatever short hashes were already assigned to MySpecialSnowflakeThingDef things, and you could end up with collisions like EldVarg has. (This is the issue reported from 1280, which was just partly covered up without fixing to preserve save compatibility)

That's not what's going on here, though. What's happening here is that because floors are designate-able, they have an implicit blueprint ThingDef that automatically gets created. During short hash assignment, that blueprint is getting the short hash that gets assigned to MineableGold when only Core is loaded, and then MineableGold has a collision and gets assigned a different short hash. So when loading a vanilla save with the mod installed, MineableGold turns into the blueprint. This much is "Working As Designed"; adding new mods to existing saves is not currently supported and this is one reason why.

All that said, my quick test says that "MineableGold" should get a short hash of 63717 and "FloorStoneChequerMarbleSandstone_Blueprint" should get 16639. Either I've misunderstood how the short hash gets calculated (I have made mistakes a couple times in the past), or there is still some problem going on.

[Zhentar]

Found my mistake. That's not a blueprint in the screenshot, it's a frame! "FloorStoneChequerMarbleSandstone_Frame" has a short hash of... 63717. There's the root cause, and it's not a bug.

[Tynan]

Beautifully explained by Zhentar!

In other news, I already totally refactored the hash giving to solve the ambiguities with Def subclassing so this sort of issue should be gone, theoretically, in A16.