niftools / blender_niftools_addon

The Blender Niftools Addon is a Blender add-on to enable import and export of NetImmese File Formats including .nif, .kf, .egm.
http://www.niftools.org
Other
386 stars 104 forks source link

Added support for KF export for Sid Meier's Pirates! #593

Closed svip closed 1 year ago

svip commented 1 year ago

@niftools/blender-niftools-addon-reviewer

Overview

This adds support for exporting KF files for Sid Meier's Pirates!.

Detailed Description

Testing

Be sure to set the Niftools Scene Panel's game to Sid Meier's Pirates! and user version and user version 2 to 0.

Manual

The basic test is to import a kf-file into Blender, immediately export the same kf-file, and place it in the game's custom directory. Run the game, and find the scene where there animation would appear.

Additional Information

Known issue: Some coordinates are slightly misaligned during import/export, and can therefore appear weird in the game, e.g. vertices moving far from their previous position and back again, giving a flickering appearance. Most of the time, the animations look fine, but more complicated animations have a risk of these quirks. Manually fixing these animations is probably an option. I have not seen the issue with animations I've worked on myself.

Also, I've mostly been "hacking" my way at this, just to get it to work. So it's possible someone else here might have a better solution for some of my additions.

Candoran2 commented 1 year ago

Also, could you attach an example nif and kf from the game to this PR? That way we have some test data. Having vanilla files also allows us to check whether they make the round-trip (import and then export) and allows for easier comparison to known 'good' files.

svip commented 1 year ago

By the way, do you prefer I force push my changes onto this PR, so it remains a single commit, or would you prefer several commits?

Candoran2 commented 1 year ago

By the way, do you prefer I force push my changes onto this PR, so it remains a single commit, or would you prefer several commits?

Either way is fine to me, just use what's most convenient to you.

svip commented 1 year ago

OK, force push it is. That makes the PR simpler.

By the way, speaking of the round trip, is there a way to do the round trip automatically (i.e. without using Blender - or at the very least control Blender to do it)?

Candoran2 commented 1 year ago

OK, force push it is. That makes the PR simpler.

By the way, speaking of the round trip, is there a way to do the round trip automatically (i.e. without using Blender - or at the very least control Blender to do it)?

Hm. I haven't done it, but probably if you use the -P option and create an appropriate Python script to run: https://blender.stackexchange.com/a/1366

svip commented 1 year ago

As for the example nif and kf files, I assume you mean putting them in testframework/unit/io/?

Candoran2 commented 1 year ago

Just creating a zip file and dropping it in the PR text should be good enough (if you go to edit the PR github will allow you to drag-and-drop the zip file).

svip commented 1 year ago

OK. I have attached 2 nif files and 2 kf files, with each kf file for each nif.

examples.zip

By the way, what do you use to determine whether the files make the round trip?

Candoran2 commented 1 year ago

OK. I have attached 2 nif files and 2 kf files, with each kf file for each nif.

examples.zip

By the way, what do you use to determine whether the files make the round trip?

Thank you. I don't check everything, but if there is an obvious visual difference in-game, you can start to manually try to figure out what the difference is between the two files. It's a lot easier than creating something new and having no reference point to how it's supposed to look.

svip commented 1 year ago

I have noticed one quirk. Walking animation with the skirts in the game, tend to have some unusual behaviour, when simply doing a round trip, as you can see in this gif.

Candoran2 commented 1 year ago

Do you want to wait with merging this PR until you can fix that or go ahead as it is?

svip commented 1 year ago

Honestly, I am not sure what to do about it. I wish Nifskope allowed me to dump the Nif tree into a text file, so I could do a diff to compare the two files, to get a better idea of where it went wrong. I'll confess, I am still relatively green to 3D modding and animation.

Candoran2 commented 1 year ago

Honestly, I am not sure what to do about it. I wish Nifskope allowed me to dump the Nif tree into a text file, so I could do a diff to compare the two files, to get a better idea of where it went wrong. I'll confess, I am still relatively green to 3D modding and animation.

Sniff allows you to convert a nif file to a json. The generated module also contains some code to convert a nif file to an xml, but I think that's not complete (since I had no reason to use it).

svip commented 1 year ago

Hm, Sniff doesn't support Pirates!'s NIF version. Unknown NIF version "10.1.0.0" ("User Version"=0, "User Version 2"=0). I'll look into the XML converter.

Edit: Maybe tomorrow, I am probably too tired right now. I can find the dumping code, but not how you load the file.

svip commented 1 year ago

Alas, in my naïveté, I thought writing my own nif/kf parser and JSON dumper would be feasible. I figured since I needn't none of the other stuff, programs like Sniff and NifSkope can do, it would be a somewhat straightforward task. Though, peeking through both programs' source code, it's pretty clear to me now, that the only thing truly consistent amongst nif files is the header string and the version number. From thereon, things deviate.

That being said, I have noticed that values for keys/translations are altered slightly through a round trip. I figured this was due to Python's float number handling. And since so many keys were altered slightly in this fashion, and yet so few instances produce this issue, it didn't seem certain it was the problem. And even if it was, how would I deal with these slight imprecisions? (I recognise the user could manually perform some fixes, but I'd prefer it if the add-on didn't create anything to fix on its own.)

I did try doing a hex dump comparison, but since the block order is slightly different, it makes that comparison a bit useless.

Candoran2 commented 1 year ago

Alas, in my naïveté, I thought writing my own nif/kf parser and JSON dumper would be feasible. I figured since I needn't none of the other stuff, programs like Sniff and NifSkope can do, it would be a somewhat straightforward task. Though, peeking through both programs' source code, it's pretty clear to me now, that the only thing truly consistent amongst nif files is the header string and the version number. From thereon, things deviate.

That being said, I have noticed that values for keys/translations are altered slightly through a round trip. I figured this was due to Python's float number handling. And since so many keys were altered slightly in this fashion, and yet so few instances produce this issue, it didn't seem certain it was the problem. And even if it was, how would I deal with these slight imprecisions? (I recognise the user could manually perform some fixes, but I'd prefer it if the add-on didn't create anything to fix on its own.)

I did try doing a hex dump comparison, but since the block order is slightly different, it makes that comparison a bit useless.

It might not be the float number handling per se, but rather the fact that we do calculations with them. One that comes to mind is the scale, which can be circumvented by setting it to 1, but others are necessary because of different ways of storing information in Blender vs nif. If you mean to 'deal with these slight imprecisions' for comparisons you could do programmatic comparisons (e.g. compute the difference and only count it if it's above a certain threshold), but their introduction might be unavoidable.

We could merge the PR as-is and come back to the glitches later - you could create an issue for them to keep track and collect the information, including example nifs/kfs.

svip commented 1 year ago

Yeah, let's merge the PR. For the most parts, it works.

I assume your suggestion about programmatic comparisons would be during import, correct? I am also kind of speculating these meshes/animations always had problems, they just happened to not show up for some reason. The game is from 2004 after all, but Morrowind is older, and still sees modding. They don't experience this?

Candoran2 commented 1 year ago

Yeah, let's merge the PR. For the most parts, it works.

I assume your suggestion about programmatic comparisons would be during import, correct? I am also kind of speculating these meshes/animations always had problems, they just happened to not show up for some reason. The game is from 2004 after all, but Morrowind is older, and still sees modding. They don't experience this?

The programmatic comparison was a suggestion as an alternative for your xml/json comparison. E.g. load both files with the generated module and then loop over the array values.

As for Morrowind, I haven't heard any complaints or seen any issues like that kind of glitch, but I also don't really work on animations - or Morrowind - that much.

svip commented 1 year ago

The programmatic comparison was a suggestion as an alternative for your xml/json comparison. E.g. load both files with the generated module and then loop over the array values.

OK, I was trying to see if I could attempt that approach. But it wasn't particularly clear to me, peeking through the code, how one would load a file, and then loop through its values. I will admit, Python is not a language I am super confident in.

Candoran2 commented 1 year ago

OK, I was trying to see if I could attempt that approach. But it wasn't particularly clear to me, peeking through the code, how one would load a file, and then loop through its values. I will admit, Python is not a language I am super confident in.

Looking at an example file (for example gov_e_govm_pc_enter_ec1000.kf). I would do something like this:

from generated.formats.nif import NifFile

kf_1 = NifFile.from_path(<path to first kf>)
kf_2 = NifFile.from_path(<path to second kf>)

diff_limit = 0.05

kf_1 = NifFile.from_path('<path to first kf>')
kf_2 = NifFile.from_path('<path to second kf>')

reference_block_controller_map = {}
for controlled_block in kf_1.roots[0].controlled_blocks:
    reference_block_controller_map[controlled_block.target_name] = controlled_block.controller

for controlled_block in kf_2.roots[0].controlled_blocks:
    # let's only consider NiKeyFrameControllers for now
    if type(controlled_block.controller).__name__ == 'NiKeyFrameController':
        reference_block = reference_block_controller_map[controlled_block.target_name].data
        query_block = controlled_block.controller.data
        # check the translations
        for r_key, q_key in zip(reference_block.translations.keys, query_block.translations.keys):
            for r_value, q_value in zip(r_key, q_key):
                if abs(r_value - q_value) > diff_limit:
                    # do some stuff to know where it is/what it is, e.g.
                    print(f'reference value {r_value} vs query value {q_value}')

Of course the code would have to be adjusted if you wanted to check rotations or scales.

Also, I just only now noticed that the PRs you made were for the master branch, which they shouldn't have been. Typically we only PR to the develop branch, and when it's release-ready we put it into master. I should have spotted that, but too late now. Oh well, we'll fix it after creating the release.

svip commented 1 year ago

Thank you for the code example. That's definitely something I can work with.

Apologies for suggesting to the master branch, it was what Github suggested when I created my PR. At least, fortunately, the changes are minor.