jmelesky / omwllf

OpenMW leveled list fixer
ISC License
38 stars 9 forks source link

How can i write other kinds of records? #4

Closed i30817 closed 4 years ago

i30817 commented 5 years ago

I'm trying to adapt omwllf code to make a mod.

https://en.uesp.net/morrow/tech/mw_esm.txt

The idea is to add a script to all the books that:

  1. are scrolls (the 3rd byte on the BKDT here is 1)
  2. have no script already
  3. have a 'enchantment' ie: they're magic scrolls.
  4. Optionally, remove the 'sell spells' from all npcs that have it (the same effect as the 'no spells for sale' mod, only it dooesn't depend on MWSE so it can work on openmw).

I have managed to parse books (welll, i just reused your code, that parses books fine), and now i wanted to test rewriting before starting to change things (adding a script... btw, can the script on a morrowind mod be a external file or does it have to be a record in the esp?).

I'm having trouble conceptualizing the stride and what methods to use to pack.

this is the little i have:


def packBook(rec):
    start_bs = 'BOOK'

    name_bs =  packStringSubRecord('NAME', rec['name'])
    model_bs = packStringSubRecord('MODL', rec['modl'])
    fname_bs = packStringSubRecord('FNAM', rec['fnam'])

    bkdt_bs = rec['bkdt']

    itex_bs = packStringSubRecord('ITEX', rec['itex'])
    scri_bs = packStringSubRecord('SCRI', rec.get('scri', ""))

    reclen = len(name_bs) + len(model_bs) + len(fname_bs) + len(bkdt_bs) + len(itex_bs) + len(scri_bs)

    reclen_bs = packLong(reclen)
    return start_bs + reclen_bs + name_bs + model_bs + fname_bs + bkdt_bs + itex_bs + scri_bs

Stuck on that BKDT subrecord with 20 bytes TypeError: must be str, not bytes when summing up the return . I didn't actually change it, ie: this is the parsing:


getRecords(f, 'BOOK')

def parseCandidateScrolls(rec):
    bkrec = {}
    sr = rec['subrecords']

    bkrec['type'] = rec['type']

    for r in sr:
        if r['type'] == 'NAME':
            bkrec['name'] = parseString(r['data'])
        elif r['type'] == 'MODL':
            bkrec['modl'] = parseString(r['data'])
        elif r['type'] == 'FNAM':
            bkrec['fnam'] = parseString(r['data'])
        elif r['type'] == 'BKDT': 
#(20 bytes float weight, long value, long (scroll == 1, not scroll  0), long SkillID (-1 no skill), long enchantpoints
            bkrec['bkdt'] = r['data']
        elif r['type'] == 'ITEX':
            bkrec['itex'] = parseString(r['data'])
        elif r['type'] == 'ENAM':
            bkrec['enam'] = parseString(r['data'])
        elif r['type'] == 'TEXT':
            bkrec['text'] = parseString(r['data'])
        elif r['type'] == 'SCRI':
            bkrec['scri'] = parseString(r['data'])
#        else:
#            print(r)

    return bkrec
jmelesky commented 5 years ago

Okay, let's see here.

First, as far as I'm aware, scripts can only be in a SCPT section in a mod, so no external files. Sorry about that, the format seems a bit hacked-together based on modern standards.

Second, the type error should be resolved if you declare the strings as binaries. Something like:

start_bs = b'BOOK'

instead of

start_bs = 'BOOK'

In python, when you x + y, it uses the type for x, and the rest needs to match. So you need to make sure that start_bs is what's needed.

Hope that helps, and sorry for taking so long to respond,

-john

i30817 commented 5 years ago

Ah, i'd forgotten i've opened this. I eventually managed to figure it out and even made some 'generic' packing and unpacking methods.

I had some trouble with the unpacking, because there are subrecords that should not be strings or should be 'lists', but there is nothing really indicating that on the fileformat (even if you make every 'doubled' type a list, there are individual subrecords that should be lists but only have one instance of that particular subrecord).

I ended up with blacklists for that, a bit clumsy but it works.

https://github.com/i30817/raremagic4openmw

I also found some strange things. For instance i'm reading strings with:

def parseString(ba):
    i = ba.find(0) # find first \0
    if i == -1:
        i = len(ba)
    return ba[:i].decode(encoding='ascii', errors='ignore')

instead of your version:

def parseString(ba):
    i = ba.find(0) # find first \0
    return ba[:i].decode(encoding='ascii', errors='ignore')

I think some 'sufficiently large string subrecords don't have \0 and are limited by max size instead.

There was also the case of the BOOK->TEXT subrecord which i found to not have a \0 at the end even if the book text subrecord has unlimited size (well the size is serialized just before the subrecord, but you know what i mean). I vbindiff compared a save of the same esp in openmw-cs and made manually to reach this conclusion so maybe my methodology is flawed

i30817 commented 5 years ago

I also decided that my mod would overwrite the previous esp instead of adding a new version with a date derived name. I think overwriting the old version is good, considering the only requirement is the omwaddon being near the end of the load order and allows me to simply ignore the previous version mod as part of load order when collecting records to alter, which is the correct behavior to prevent bizarre errors.

I'm.... about 95% sure you should do something similar to warn/panic/ignore previous versions of the levelled list omwaddon spoiling your new one instead of depending on users being smart enough to deactivate the previous version before generating a new one.

jmelesky commented 5 years ago

Oh, that string length thing is a good catch. With your permission, I'll add it to my code.

Like you, I was operating off of internet documents which reverse-engineered the format, so there are likely more issues of that nature. The double/list issue strikes me as the same family of problems. The solution? Write a new, updated file spec, of course! Volunteers requested. :)

As for overwriting instead of creating, one of my goals is to work with multiple sets of mods without issue (hence the ability to specify a different conf file at the command line). I spit out text at the end of the run explicitly telling people to uncheck other omwllf mods, but, agreed, that's not the best solution.

Instead, I think the "right" way for this tool is to edit the conf file directly -- no need to check/uncheck anything. One of the reasons I haven't tackled issue #2 yet is because it quickly expanded, in my mind, to include the need to check for other omwllf mods, which expanded to changing the conf file. I really ought to break those tasks out so I can tackle them more easily.

i30817 commented 5 years ago

Oh, that string length thing is a good catch. With your permission, I'll add it to my code.

Yeah, it's fine ofc.

About issue 2, i dunno what's the best solution. In fact i'd swear openmw-launcher adds mods in that directory to the openmw.ini if you enable them, so i don't really see a problem there? There is probably some extra functionality in openmw i'm missing.

jmelesky commented 4 years ago

Closing -- old and now only referencing #2.