Hacking the iesh

[critto]

I thought I'd start a separate thread in here for any further hacking related to iesh. We're already way out of modding Q&A scope back in the old thread.

 

I've been implementing indexing of the override folder to browse the data that is not present in vanilla BIFFs. While doing so, I've encountered that the SPL format fails sometimes:

Traceback (most recent call last):
  File "./iesh", line 308, in <module>
    exec (current_command)
  File "<string>", line 1, in <module>
  File "/Users/mac/Documents/code/gemrb-ie_shell/infinity/builtins.py", line 67, in load_game
    obj = FileStream ().open (file_path).load_object (file_type[0])
  File "/Users/mac/Documents/code/gemrb-ie_shell/infinity/stream.py", line 288, in load_object
    obj.read (self)
  File "/Users/mac/Documents/code/gemrb-ie_shell/infinity/formats/spl.py", line 391, in read
    self.read_feature_block (stream, off, obj)
AttributeError: 'SPL_Format' object has no attribute 'read_feature_block'

It seems that read_feature_block is unimplemented completely, but I see no harm in replacing it with read_feature (), seeing as the feature block structure should be the same.

diff --git a/infinity/formats/spl.py b/infinity/formats/spl.py
index 568f9b6..f39c7da 100644
--- a/infinity/formats/spl.py
+++ b/infinity/formats/spl.py
@@ -388,8 +388,8 @@ class SPL_Format (Format):
         off = self.header['feature_block_off'] + self.header['casting_feature_ndx'] * 48
         for i in range (self.header['casting_feature_cnt']):
             obj = {}
-            self.read_feature_block (stream, off, obj)
-            self.casting_feature_block_list.append (obj)
+            self.read_feature (stream, off, obj)
+            self.casting_feature_list.append (obj)
             off = off + 48

Works like a charm. Tested it on all spells that have global feature blocks in my game, haven't encountered any problems. Are there any gotchas I'm not seeing?

 

Second question. The current implementation attempts to search the override but only for files that were indexed in BIFFs. Would it be appropriate to build a separate index for stuff in the override and do a look up on it before any attempt to locate the requested resource in the BIFF index and then checking for it in the filesystem (since there's also the cache folder which is not taken into an account yet).

 

Another good idea I am thinking about is telling iesh to fetch file from override or specifically from biff index. This would allow to browse different versions of it and, potentially, even performing a diff between the versions (which is my longterm goal here).

[critto]

STOR v1.0 had incorrect signature in register_format call, the files couldn't be browsed because of it. Is it intentional? Looked more like a typo to me.

diff --git a/infinity/formats/stor_v10.py b/infinity/formats/stor_v10.py
index 1e426a4..8fb6ab3 100644
--- a/infinity/formats/stor_v10.py
+++ b/infinity/formats/stor_v10.py
@@ -301,5 +301,5 @@ class STOR_V10_Format (Format):
         self.print_list ('cure')
 
 
-        
-register_format (STOR_V10_Format, signature='STOR V1.0')
+
+register_format (STOR_V10_Format, signature='STORV1.0')

[critto]

Could you perhaps host iesh on github to allow for better collaboration and contribution?

[SyntaxError]

It is hosted on github... where were you getting it from?

[critto]

https://sourceforge.net/p/gemrb/ie_shell/commit_browser

 

This repository is more up to date.

[critto]

I think there's a side effect to using python lists for storing strrefs. A value of -1 is treated as reference too and the value is taken from the end of the list. I think this should be fixed since this behaviour does not conform to the format. STRREF only should be a positive integer.

 

I propose a simple fix:

diff --git a/infinity/format.py b/infinity/format.py
index 42de016..bfcf8f8 100644
--- a/infinity/format.py
+++ b/infinity/format.py
@@ -517,7 +517,7 @@ class Format (object):
             elif rec_type == 'CTLID':
                 try: value2 = '(' + '0x%08x' %value + ')'
                 except: pass
-            elif rec_type == 'STRREF' and core.strrefs:
+            elif rec_type == 'STRREF' and core.strrefs and value >= 0:
                 try: value2 = '(' + core.strrefs.strref_list[value]['string'] + ')'
                 except: pass
             elif rec_type == 'RGBA':

[critto]

Right, we definitely need github. One single-day train ride, here're the changes that are present now in my local copy (everything written above and in the previous thread included):

1) support for override dir:

- gets indexed on load_game()

- survives save()/restore()

- documentation updated with examples and desc.

- export_object() supports exporting from override (ignore_override is added to export stuff straight out of game data archives)

2) added find_objects() built-in that performs look up in the override and biffs (filtered by name and, optionally, type), prints the output into the shell (I find it more useful and readable than catching exceptions from load_object())

3) load_object() now searches in override, file system and biffs (flag ignore_override is added as well)

4) strrefs are resolved and shown only if the value is positive

5) fixed a bug with IDS file names in format descriptions never being resolved properly and actually loaded for reference in printme()'s

6) ITM_V1 format:

- kit usability masks, enum per byte of DWORD

- described item types and weapon proficiencies as enums

- some cosmetics

- fix reading in global casting features from files

7) BAM & image.py:

- my new version of PIL does not support tostring() and fromstring() anymore, replaced with tobytes() and frombytes() [this needs either a straight-up fix or some sort of backwards compatible solution]

7) streams:

- signed dwords are displayed properly

- ResourceStream now tries to resolve file type parameter into RESREF from string to #FIXME some uglies

- some extra handling of exceptions is added

- OverrideStream is implemented for looking up stuff in the override folder

8) removed many trailing spaces (it's mentioned in TODO, so I did not reconfigure my code editor)

 

I have also written out some possible bugs and stuff I'd like to discuss in greater detail before attempting any patches.

[lynx]

Ok, please split stuff up into neat little commits, smallest sensible wholes as possible. You can use git commit -p in case you didn't know. Any whitespace changes should be done separately, not to pollute actual changes. On the example of the patch from the other thread: that's clearly 3 commits.

 

sto: definitely a typo

 

I think there are more folders that have override properties, but never heard of anyone using them (sounds, scripts, portraits).

 

By now it's completely ok if we go full python3.

[critto]

 

 

Ok, please split stuff up into neat little commits, smallest sensible wholes as possible. You can use git commit -p in case you didn't know. Any whitespace changes should be done separately, not to pollute actual changes. On the example of the patch from the other thread: that's clearly 3 commits.

Absolutely, I wouldn't have it any other way. Everything will be as small and atomic as possible, don't worry about that.

 

 

I think there are more folders that have override properties, but never heard of anyone using them (sounds, scripts, portraits).

I have neither. These could be included but I don't consider this a top priority.

 

 

By now it's completely ok if we go full python3.

Well, I am not very proficient in python, so I doubt that I have used something extraordinaire. To be honest, I have never worked with Python3, all my practical experience involves 2.x If you insist on Python 3 and there are any gotchas that I need to be aware of, let me know and I'll look it up. I'll try to get Python3 installed (Mac OS comes with pre-installed Python 2.7 and it's not really convenient to mix in with stand-alone package managers like Brew but I'll figure it out) and test everything once again before uploading anything. I'll have some time anyway before you get the github version up-to-date.

[lynx]

I mentioned python3 just because of your PIL comment — it sounds like it is for v3. No need to bother.

 

The repo is already up to date.

[critto]

OK, I've pushed some stuff. Once we deal with that I'll post some more (override folder, etc.).

 

Now, I had some questions regarding the ITM_V10 format (all related to fixme's):

 

1) why do you have offsets 0x25 and 0x27 of the header listed separately? According to IESDP, those are bytes reserved for Min Level and Min Strength (both are Words and take up two bytes each).

 

2) the id_req field of extended header. There's some information that it does not work?

 

3) feature_ndx, I don't think it needs a "fixme", while it's listed as an offset it's common knowledge that it's a position in the list of casting effects, not an actual offset further into the file (as you surely know).

 

4) the projectl.ids / missile.ids issue. This is quite interesting. I did some digging and, apparently, there's not strict consensus on which file to use. Maybe it needs to be looked up in both?

 

5) opcode number references a "effects.ids" enum which does not exist in any IE game that I've checked. This needs a custom ENUM that is dependent on the actual game that is loaded right now. I saw some support for game_type in the code but it seemed rather rudimentary and I didn't explore it in depth.

 

6) an unrelated question about IDS in general: quite usually the IDS files contain duplicate records that produce lots of annoying warning when you print out the file. Maybe an option needs to be introduced that suppresses these warnings?

[lynx]

1) things like this are usually just missing info. Here it is less clear, since neither strength nor level will ever exceed the byte max. Even in gemrb we just ignore them while reading.

 

2,3) no, all those fixmes can go. Besides the main lore value, this is also inspected when checking equippability.

 

4) didn't look into it, but the main thing appears to be the offset by 1 and resref vs name.

 

5) gemrb/iesdp, eg. gemrb/unhardcoded/iwd2/effects.ids; yes, this implies a dependency and we need to deal with this better. Judging from our history, these hardly change now, so duplication (keeping in sync) would not be terribly problematic.

 

6) I guess.

[critto]

4) didn't look into it, but the main thing appears to be the offset by 1 and resref vs name.

 

OK, I'll study this further later.

 

 

5) gemrb/iesdp, eg. gemrb/unhardcoded/iwd2/effects.ids; yes, this implies a dependency and we need to deal with this better. Judging from our history, these hardly change now, so duplication (keeping in sync) would not be terribly problematic.

 

OK. I'll checkout the gemrb version, then I'll try to prep those lists for BG1/2 and the EEs, and see about implementing them.

 

There's one more thing that's been annoying me:

Cmd: restore("bg2ee")
Cmd: o = load_object("demogor2", 1009, ignore_override=True)
Cmd: quit
None
(<type 'exceptions.AttributeError'>, AttributeError("'NoneType' object has no attribute 'close'",), <traceback object at 0x1087d34d0>)

When you load something from the BIFF file, it gets cached and the object is not taken care of properly at program termination. I did some debugging, and it appears that the Stream class is cleaned up before close() methods are called on all of those instances cached in a list. I'm not sure how to affect that in any way other than provide an explicit tear-down method that is called when user terminates a iesh session.

[lynx]

Doesn't iesh lookup in the ids automatically? If that's not the case yet, it should — there's no reason to add in total ~1000 list members manually. So just copy the ids files where it can find them (perhaps a new search path is needed, so they can be with the sources). EEs are just a matter of copying the originals over and appending the new opcodes.

[critto]

Yes, it does, however, it looks for IDS inside the game data via ResourceStream (it has just occurred to me as well that I might've forgotten to add lookup for ids files in the override, I'll check that too). So patching is needed anyway if we want to ship effects.ids with iesh itself.