Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can i write other kinds of records? #4

Closed
i30817 opened this issue Nov 15, 2018 · 6 comments
Closed

How can i write other kinds of records? #4

i30817 opened this issue Nov 15, 2018 · 6 comments

Comments

@i30817
Copy link

i30817 commented Nov 15, 2018

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
Copy link
Owner

jmelesky commented Dec 3, 2018

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
Copy link
Author

i30817 commented Dec 3, 2018

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
Copy link
Author

i30817 commented Dec 3, 2018

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
Copy link
Owner

jmelesky commented Dec 3, 2018

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
Copy link
Author

i30817 commented Dec 3, 2018

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
Copy link
Owner

Closing -- old and now only referencing #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants