xoreos / xoreos-tools

Tools to help the development of xoreos
https://xoreos.org/
GNU General Public License v3.0
68 stars 29 forks source link

xml2gff: NWN:EE invalid file format #71

Closed drake127 closed 3 years ago

drake127 commented 3 years ago

We migrated to NWN:EE and our servervault (converted by xml2gff) completely broke with following issue:

*** ValidateGFFResource sent by user.
*** FAULT *** ValidateSafePointersInData.  struct (1) was referenced more than once.

I believe that's because of deduplication of referenced fields that's apparently not allowed in NWN:EE. Can I ask at least for pointers how to fix this issue?

DrMcCoy commented 3 years ago

Hmm, we don't really dedup on structs. Or at least, we shouldn't.

Do you still have the original data before throwing it into xml2gff and try it with a recent xoreos-tools version (0.0.6 and/or the current master branch)? If you had used xml2gff from an earlier version (i.e. from a checkout between 0.0.5 and 0.0.6), I'm afraid a bug in the GFF3 writer created faulty files in some cases. I don't think the files are repairable then, unfortunately.

DrMcCoy commented 3 years ago

If it does still happen with xoreos-tools 0.0.6, can you attach an example file? Pre manipulation with the xoreos-toosl and post, ideally, so I can see what happens.

drake127 commented 3 years ago

It happens with latest version and it's easily (always) reproducible.

I don't think these files are actually broken as GFF Editor (and old NWN) opens them just fine. XML files also look good, they are just not reconstructed properly for NWN:EE.

Not even basic character stats are recovered, so I assume that some basic check is failing. I am now trying to minimize XML file that still breaks the game to be able to fix in manually in hex editor.

broken-char.zip

Note that we are using cp1250.

drake127 commented 3 years ago

Good news (somewhat): I believe that adding a \<list> actually breaks the format with given error message.

Following file emits the error:

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<gff3 type="BIC">
  <struct id="4294967295">
    <list label="ClassList">
      <struct id="2">
        <sint32 label="Class">0</sint32>
        <sint16 label="ClassLevel">1</sint16>
      </struct>
    </list>
  </struct>
</gff3>
drake127 commented 3 years ago

I have created similar (hopefully same) file in GFF Editor and it works. There are some differences if you want to take a look: minimal.zip

DrMcCoy commented 3 years ago

Thanks, I'll probably have a look in the evening

drake127 commented 3 years ago

I now believe that structs inside a lists are not supposed to be in field list. They cannot have label either (because they are not in the field list). Structs which are part of other structs can and will be present in field list.

I am going to run some tests now.

drake127 commented 3 years ago

I tested it on ~10k characters in our NWN servervault and it's really the case. I submitted example PR but feel free to implement it however you like, it's rather simple change.

DrMcCoy commented 3 years ago

Huh, nice, interesting, thanks! Original NWN itself still eats the files like that, yes?

drake127 commented 3 years ago

I didn't try yet. I verified that .xml files don't differ and that NWN:EE doesn't complain (and NWN:EE parses NWN bic files well). Also GFF Editor and modified xml2gff got me exactly same results.

DrMcCoy commented 3 years ago

And I Just tried a gff2xml -> xml2gff -> gff2xml roundtrip, both with and without the patch, results are identical. And old NWN seems to at least eat a localvault character without any issues. Nice :)

DrMcCoy commented 3 years ago

Merged #72, closing this issue, thanks! :)