collinsmith / riiablo

Diablo II remade using Java and LibGDX
http://riiablo.com
Apache License 2.0
884 stars 101 forks source link

StringTBL duplicate keys + performance improvement #25

Open collinsmith opened 5 years ago

collinsmith commented 5 years ago

I noticed that strModEnhancedDamage contains two references in patch_d2.mpq -- data\local\lng\eng\patchstring.tbl which is causing an issue because the result from the first key is returned, which is incorrect. The first key ends with a new line character while the second key does not. From what I've seen in item mods, the first key should be junk and the second key is what we actually want.

In the meantime I've disabled that specific problem key within StringTBLs by changing its strOffset--, which effectively reduces the key's string length by 1 and means that when comparing the key length is will only contain strModEnhancedDamag and no longer match, so the second key will return as expected. In other words, I've broken the first key on purpose.

Long term I would like to (and I think this is representative in the original design), search for existing keys when a StringTBL is loaded and basically overwrite them/disable existing keys which brings me to my second point:

Looking at how StringTBL lookups are performed, it might be easier to compare the keys using their hashes instead of comparing their content (or make the hash comparison a fail-first check). Some tests may need to be performed to see if all the keys are unique, or unique enough, but this may provide a decent performance boost if they are indeed unique. I have seen cases though where the lookup needs to iterate through the table a bunch, so this may mean all those iterations are of keys with equivalent hashes, meaning my approach is misguided.