Concerning v1.33b19

[Taimon]

I have been tinkering with v1.33b19 lately (devSin's version precisely) and intend to use this thread as some kind of feedback/question/bugreport platform.
It's a bit sad that v1.33 never got a proper release, since the codebase changed a lot compared to 1.32. Lets hope Fred finds his way back into the forums.

Anyway, the first thing that really annoyed me was, that, if you have a lot of cre files in override, and you start up NI and click on a dialog file, NI seems to freeze for some time (40s on my install). It is creating a script compiler instance which in turn gathers all creature and area names for later verification. I tried to fix this locally by moving the indexing in a SwingWorker (requires Java 1.6 unfortunately) and deferred script compilation until you click compile (or simply view the relevant code in an edit view when "Autocheck BCS" is enabled). Also added some notification in the status bar.

I noticed another small compiler/decompiler bug: When compiling an Integer, NI does some kind of unsigned -> signed conversion, if the value is too big for a signed int (> 2^31 - 1). This is done by subtracting 2^32. Upon decompilation this is reversed, but the condition was faulty (only checked for < 0) and so compile/decompile of "HappinessLT(Myself, -289)" would lead to unexpected results.
However, is this whole conversion really needed? I'm not so firm with scripting engine details.

Is there a list of release-critical bugs in v1.33b19? I may spend some time trying to fix some of them.

Ah, and if anyone is interested in diffs or something, just let me know.





 

[devSin]

Anyway, the first thing that really annoyed me was, that, if you have a lot of cre files in override, and you start up NI and click on a dialog file, NI seems to freeze for some time (40s on my install). It is creating a script compiler instance which in turn gathers all creature and area names for later verification. I tried to fix this locally by moving the indexing in a SwingWorker (requires Java 1.6 unfortunately) and deferred script compilation until you click compile (or simply view the relevant code in an edit view when "Autocheck BCS" is enabled). Also added some notification in the status bar.

This should occur regardless of the override contents (file access will be slower, but meh, it has to be done)? Trolling through to get the script names is lame, but it should only happen once, so the damage is fairly minimal (less so than upping the version requirement yet again, at least). It shouldn't launch into its full gathering until you actually hit a node with triggers/actions (e.g., view a dialogue with code for State 0 or switch to edit view and browse the action/trigger list), but the decompiler does need it for its compile/decompile check (to populate the warnings list with invalid script names). Deferring isn't necessarily bad, but you have to spend the time somewhere, and I can't think of many cases where building the list early is untenable (even if you don't need it right then, you will eventually when looking at code that references script names, even if not changing anything, or if you want to run the BCS checker).

That 40s is dreadful, though (40ms, maybe, hopefully?). With ~2500 CREs in the override (not to mention hundreds of AREs), it'd be rare for me to see a 4s delay opening when opening the first script or dialogue.

I noticed another small compiler/decompiler bug: When compiling an Integer, NI does some kind of unsigned -> signed conversion, if the value is too big for a signed int (> 2^31 - 1). This is done by subtracting 2^32. Upon decompilation this is reversed, but the condition was faulty (only checked for < 0) and so compile/decompile of "HappinessLT(Myself, -289)" would lead to unexpected results.
However, is this whole conversion really needed? I'm not so firm with scripting engine details.

Probably not, but there may be some purpose to it (the sign/unsign conversion is done in other places, so I just decided not to bother even thinking about it). In BG2, the BCS ints (all of them, I think) are signed, which is why you'll get some -1 values spit out as 2^32 (even though the compiled script gets the signed value). Stupid in lots of cases, but not so bad when working with ORd state values (otherwise, you have to do the conversion yourself).

[Taimon]

My description on the first problem was really inaccurate. You are right on most parts. It only happens if the script compiler is involved (i.e. the dialogue file has triggers or actions and you are viewing them) and there is no instance already.

Sadly, the 40s are true for my older system. (Harddisk is 3 years old and I'm still using Win2k.) I have like 3500 cre files in override. (I know, I should biff them.) Thinking about it, when the average access time is ~10ms and the harddisk doesn't do some tricks like waiting for multiple request and answer them in one turn, the 40s seem appropriate for 3500 files. Windows does have some kind of file cache, so if you close NI and reopen it, the whole procedure is much faster. (I actually did write some test code and it took only half a second if the files are cached.)
The main reason for the deferring was, that it really takes that long the first time. If it only took 3 seconds, I wouldn't mind building the list early.
Maybe I rewrite the indexing thread, so that it doesn't need SwingWorker. (It just was the quickest solution.)

I'm not so sure what to do about that unsign/sign conversion. Will investigate further, if I find some time to do so. For the moment I just disabled the signed -> unsigned conversion in the script decompiler (which strangly is only applied if there is some IDS mapping in the action which uses the integer).

[devSin]

The problem with dialogues is that the code is all stored as plain (not compiled) text -- if you disable the compile/decompile check when assembling the Viewer components, it's going to be fairly rare that the dialogue code is ever checked (unless you hit Compile Check when writing or changing your dialogues, you're never going to know if something is wrong). It works for Autocheck BCS since the code has to be decompiled to be displayed anyway (and you must recompile to save any changes), but dialogues would happily let you write nonsense code without any indication that anything is wrong if they don't force the comp./decomp. routine. I also don't know how disabling the auto-compilation would affect the dialogue code checker or the reference searcher (although, the reference searcher doesn't actually pick up references in dialogue triggers/actions, so I don't think it'd make much difference there).

Only disabling the conversion of nr to a positive value will cause some failure with giant numbers (the (de)compiler will barf if it encounters a large unsigned DWORD in something like a combined state check, for instance). I haven't looked into other places where it might fail, so it might require additional changes elsewhere (or special handling for code that commonly has negative numbers, but only as a last resort).

[Taimon]

Well, I thought, that if you want to save your changes to a dialogue file, you first have to click "Update". And this should automatically invoke the check compile. But nevertheless, I now see your point with doing the compile/decompile on the viewer, since the decompile does transformations like replacing the number on a state check with the ORed strings from state.ids. (You mentioned it before, but stupid me didn't understood it then.)
I'm still wondering why the "indexing" on your computer is so much faster.

Concerning that (nr < 0) check in Decompiler.decompileInteger: I re-enabled it, and put it in the next if statement (that checks if this is a flag value like state). Solved the HappinessLT issue and hopefully doesn't break anything else.

[devSin]

Concerning that (nr < 0) check in Decompiler.decompileInteger: I re-enabled it, and put it in the next if statement (that checks if this is a flag value like state). Solved the HappinessLT issue and hopefully doesn't break anything else.

OK, finally had a look at this. Since everything here is bitwise, the signedness doesn't really matter. You should be able to get rid of the sign conversion entirely and replace the IDS walk loop condition with nr != 0 && bit < 32 (~L:267) and the "your IDS is bahroken!" condition with nr != 0 (~L:279).

[Taimon]

Yep, this is probably the best solution.
I also changed the unsigned &rarr; signed conversion in compileInteger to a simply (int) typecast.
(Same could be done for the writeUnsigned* methods in util.Filewriter. I'm not even sure the current check there is correct.)

Btw, someone should start commenting Near Infinity.

[devSin]

I also changed the unsigned &rarr; signed conversion in compileInteger to a simply (int) typecast.

I: params in BCS need to be signed 32-bit ints, so the conversions here won't ever break anything (not much reason to just cast to int, and it all has to be ASCIIized anyway).

(Same could be done for the writeUnsigned* methods in util.Filewriter. I'm not even sure the current check there is correct.)

Actually, the conversions there should be totally pointless with an explicit data size (e.g., any single-byte value - 256 is going to give you the same byte as the original value)? It shouldn't be making any difference in the value written in the few places it's used (IIRC, writeLong() is consigned to only a few datatypes), so probably not worth changing.

[Taimon]

I: params in BCS need to be signed 32-bit ints, so the conversions here won't ever break anything (not much reason to just cast to int, and it all has to be ASCIIized anyway).

I just found

Code: [Select]

// unsigned -> signed conversion
return String.valueOf((int) Long.parseLong(value));
easier to read than

Code: [Select]

long nr = Long.parseLong(value)
if (nr >= 2147483648L) {
  nr -= 4294967296L
  return String.valueOf(nr);
}
else
  return value;
So its only aesthetics.

Actually, the conversions there should be totally pointless with an explicit data size (e.g., any single-byte value - 256 is going to give you the same byte as the original value)? It shouldn't be making any difference in the value written in the few places it's used (IIRC, writeLong() is consigned to only a few datatypes), so probably not worth changing.

Yep.
That's why I wanted to remove the check, because the typecast already takes care of this. (With a possible exception with that writeUnsignedThree). But you are right: it's probably not worth changing.
Some should point me to the real problems.

[devSin]

CHR->CRE conversion is disabled and broken, if you want to fix it. It basically exists to convert a CHR (where the engine puts structures that change at the bottom) to a CRE (where the toolset/editor order structures in a more ordered fashion).

There's also a bug for BAM playback when switching to a cycle with no image data when the BAM is animating (the play button gets stuck enabled and can't be disabled until you switch back to a cycle with > 1 frames) that I haven't looked into yet.

[Taimon]

Huh? Somehow I did not see your last post until now. Will take a look at those two things.

But I actually had a question about the biff editor: Is it intended that only files from the chitin.key are listed there?
Someone may have biffed the override and you will actually delete files from that bif if you copy a single file into it.

[devSin]

The BIFF editor (and key cleanup) is so irredeemably evil that I'd just as soon not touch it. :-)

But I thought all override files get listed in the right panel (with BIFF contents only in the left panel)?

A resource kinda has to be in the key if it's in the BIFF or else there's no real way to account for it (we're certainly not going to walk the BIFF to see if all the key entries comprise the entire BIFF).

[Taimon]

Okay, here's the deal: Someone had installed IWD casting graphics from BG2 Tweaks but didn't like it and wanted to revert to the standard animations. He had biffed the override (Mega install or something). I don't think there is a way to get the original casting graphics from spelanim.bif with Near Infinity now. (besides undoing the override bif, which made a backup of chitin.key)

[devSin]

If they're the same name, sure. NI (and the game) can only go on what the key says, and it now says those resources are in the custom override BIFF (as far as they're concerned, the originals don't even exist).

He could always restore the original key, grab the original files and dump them in the override, and then go back to the old key, but there's not really some way to work around the fact that the key is *the* index of BIFF contents, and we pretty much have to look where it says to look for any particular resource.

[Taimon]

Yeah, you are right, forget it. I thought we could somehow get the names of the files in the BIFF without a KEY file, but this is obviously not the case. Thanks for your answers.

[Taimon]

Do you have a example BAM for that playing button bug?

[devSin]

Not offhand (you'd have to find one that has multiple cycles but no frames for later cycles). I can't remember which BAMs they were, and I'm not currently working with BG2.

[Taimon]

Nevermind, I simply changed the frame count for one cycle to zero to reproduce.
Looks like a very minor GUI issue.

Code: [Select]

--- infinity/resource/graphics/BamResource.java
+++ infinity/resource/graphics/BamResource.java
@@ -102,11 +102,22 @@
     }
     else if (event.getSource() == bnextanim) {
       selectedAnim++;
+      // stop timer if no frames in cycle
+      if ((timer != null) && timer.isRunning()
+          && (anims[selectedAnim].frameCount == 0)) {
+        timer.stop();
+        bplay.setSelected(false);
+      }
       selectedFrame = 0;
       showFrame();
     }
     else if (event.getSource() == bprevanim) {
       selectedAnim--;
+      if ((timer != null) && timer.isRunning()
+          && (anims[selectedAnim].frameCount == 0)) {
+        timer.stop();
+        bplay.setSelected(false);
+      }
       selectedFrame = 0;
       showFrame();
     }


[Taimon]

I did some hackery to allow undoing multiple dialog file conversations. Might have some side effects on memory usage, though.

Code: [Select]

--- infinity/resource/dlg/Viewer.java
+++ infinity/resource/dlg/Viewer.java
@@ -52,6 +52,7 @@
   private State currentstate;
   private Transition currenttransition;
   private boolean alive = true;
+  private DlgResource undoDlg;
 
   Viewer(DlgResource dlg)
   {
@@ -142,15 +143,27 @@
     bundo.setEnabled(false);
   }
 
+  public void setUndoDlg(DlgResource dlg) {
+    this.undoDlg = dlg;
+    bundo.setEnabled(true);
+  }
+
 // --------------------- Begin Interface ActionListener ---------------------
 
   public void actionPerformed(ActionEvent event)
   {
     if (!alive) return;
     if (event.getSource() == bundo) {
+      if(lastStates.empty() && (undoDlg != null)) {
+        showExternState(undoDlg, -1, true);
+        return;
+      }
       State oldstate = lastStates.pop();
       Transition oldtrans = lastTransitions.pop();
-      bundo.setEnabled(lastStates.size() > 0);
+      if (lastStates.empty() && (undoDlg == null)) {
+        bundo.setEnabled(false);
+      }
+      //bundo.setEnabled(lastStates.size() > 0);
       if (oldstate != currentstate)
         showState(oldstate.getNumber());
       if (oldtrans != currenttransition)
@@ -202,16 +215,7 @@
         else {
           DlgResource newdlg = (DlgResource)ResourceFactory.getResource(
                   ResourceFactory.getInstance().getResourceEntry(next_dlg.toString()));
-          alive = false;
-          if (NearInfinity.getInstance().getViewable() == dlg)
-            NearInfinity.getInstance().setViewable(newdlg);
-          else {
-            ViewFrame frame = (ViewFrame)getTopLevelAncestor();
-            frame.setViewable(newdlg);
-          }
-          ((Viewer)newdlg.getDetailViewer()).showState(currenttransition.getNextDialogState());
-          ((Viewer)newdlg.getDetailViewer()).showTransition(
-                  ((Viewer)newdlg.getDetailViewer()).currentstate.getFirstTrans());
+          showExternState(newdlg, currenttransition.getNextDialogState(), false);
         }
       }
       if (alive) {
@@ -323,6 +327,26 @@
         transTriList.add((ResponseTrigger)entry);
       else if (entry instanceof Action)
         actionList.add((Action)entry);
+    }
+  }
+
+  private void showExternState(DlgResource newdlg, int state, boolean isUndo) {
+
+    alive = false;
+    if (NearInfinity.getInstance().getViewable() == this.dlg)
+      NearInfinity.getInstance().setViewable(newdlg);
+    else {
+      ViewFrame frame = (ViewFrame)getTopLevelAncestor();
+      frame.setViewable(newdlg);
+    }
+    Viewer newdlg_viewer = (Viewer) newdlg.getDetailViewer();
+    if (isUndo) {
+      newdlg_viewer.alive = true;
+    }
+    else {
+      newdlg_viewer.setUndoDlg(this.dlg);
+      newdlg_viewer.showState(state);
+      newdlg_viewer.showTransition(newdlg_viewer.currentstate.getFirstTrans());
     }
   }
 


[Taimon]

Again some hacks to further improve "navigating" in the resource tree.
Introduced "crosslinks" in scripts to quickly get to the relevant resource.
(For example right-clicking on the spell in ForceSpellRES should bring you to the SPL file.)

[cmorgan]

Are these integrated into the SourceForge materials, or should I (beg) PM you for a copy of the tinkered-with NI in .jar?

[Taimon]

I didn't commit anything on sourceforge. (Not sure, if I'm even allowed to do that.)
PM me your mail address or I can upload it somewhere, if you want to.

Note that these changes are only hacks and not solid code.
It's mostly usability stuff for me.

I haven't done a changelog either, so who knows what else I changed.

[Miloch]

Well, you're just continuing the tradition of previous developers .  I think it's GNU GPL, so you can mod and upload it as you like (as long as you include the GNU license and whatever else it requires).  Someone should probably just host this somewhere properly (on a site that doesn't have persistent latency issues, if such exists).

[Taimon]

jar
src

Needs Java 1.6 (SwingWorker). No support, sorry.

Changes as far as I can remember:

• moved scriptname indexing in a SwingWorker, to not block the gui
• messed a bit with the "unsigned -> signed" conversion
• fixed the two things devSin posted (BAM playback + CHR-CRE converter)
• implemented the mis-index effects checker requested by CamDawg
• introduced tlk charset option for our friends with polish tlks
• added undo dialog path over multiple dlg files
• added crosslinks to the scripts textareas (also in dlg, trigger by right-click)

[cmorgan]

Thank you! Nabbed it...