Possible new version of SCI Companion

[Collector]

Actually, I have absolutely nothing against the undithering feature (other than preferring the original dithered look personally). I was excited to see how it could develop further and be closer to how the original dithering was perceived. I admired him for it! I and others would never have spoken against it at all in the first place had things happened differently. What turned me off was the enforcing of it to be the default option and the whole pompous attitude he had about it, claiming that it was exactly what Sierra, the artists, and everyone wanted and then proceeding to belittle me and avoid my arguments against enforcing it as the default option when I actually made valid points and twisted everything I said. Which just made the debate continue in circles needlessly with my trying to correct his misconceptions of my opinions and getting the subject back on track. The man was arrogant about it to say the least. To this day the undithering checkbox is not and undithering checkbox at all but a dithering checkbox which is enabled to "add" dithering, like it was never there to begin with. Just more evidence of that attitude. I understand the technicality that technically the dithered colours are their own values and could be represented as anything, but his assertion that those colours generated by his algorithm were actually something like what the artists were seeing when they drew these backgrounds and that Sierra created drivers that translated that down into the standard dithered 16-colour palette was just plain asinine.

Oh, I know. It wasn't just you. He was trying to belittle everyone that question having it forced on the user. No one was arguing it be removed. In the end it does not matter since as you note, it was dropped as default. It is over and done with.

Actually, it seems like all work on the SCI side of things just stopped cold suddenly for some reason.

It is mostly because they haven't made progress with SCI32, whether that is because they don't have anyone working on it and it has been shelved or it just has them stymied. Too bad as the few SCI games that I would like to be supported by the project are the Win only games. Let's face it. The FreeSCI project gave them a big leg up with SCI, but that help ended with SCI1, given that they had started on it before it was merged into SVM.

[MusicallyInspired]

Troflip, I can't remember if I mentioned this or not and you had seen it. In SCI Companion 2.1.0.5, text resource lines can only be 259 characters long, yet in SCI Studio they can be as long as you want. Has this been fixed/were you aware of it? Also, the Windows copy/paste/cut shortcut keys don't work while editing text resource lines, even though you can right-click and perform these actions in the context shell menu.

One more thing, the text resource editor in SCI Studio has an entry for text line entry "names". Is there a purpose to these or are they used at all by the interpreter? Or is it merely just to make finding things easier?

[troflip]

Troflip, I can't remember if I mentioned this or not and you had seen it. In SCI Companion 2.1.0.5, text resource lines can only be 259 characters long, yet in SCI Studio they can be as long as you want. Has this been fixed/were you aware of it? Also, the Windows copy/paste/cut shortcut keys don't work while editing text resource lines, even though you can right-click and perform these actions in the context shell menu.

One more thing, the text resource editor in SCI Studio has an entry for text line entry "names". Is there a purpose to these or are they used at all by the interpreter? Or is it merely just to make finding things easier?

Yeah, I went through the forums and looked for SCI Companion bug reports when I picked this back up. The character limit has been addressed (or at least made very large).

The cut/copy/paste not working is super annoying, and I've fixed it just now. It's amazing all the hoops you have to go through to get basic stuff working like that in MFC (MFC has its own command system, and I have cut/copy/paste commands for general usage throughout SCI Companion, and they were blocking it from working in the edit control in the list - so I have to hook the keystrokes and forward them to the edit control when its active).

I just looked at the name thing in SCIStudio... it doesn't look like the names are even saved (if I save the text resource and re-open it, they are gone). So I'm not sure what its purpose is?

[MusicallyInspired]

Wow. It IS amazing how much goes on under the hood. Thank you very much for the fixes.

[gumby]

Troflip, did you happen to address the number of cases a switch statement can have?  I think it topped out at less than 20.  Assembly support would be great too, though I suppose this is a non-trivial feature.

[troflip]

Troflip, did you happen to address the number of cases a switch statement can have?  I think it topped out at less than 20.  Assembly support would be great too, though I suppose this is a non-trivial feature.

This thread suggests the limit was with SCIStudio, not SCI Companion:
http://sciprogramming.com/community/index.php?topic=588.0

asm support isn't too difficult.

[lance.ewing]

I might do that eventually. In a couple of the recent examples I've come across, the actual correct code isn't too hard to figure out when looking at it by hand. It's just gaps in my decompilation algorithms. But I have encountered some truly perplexing byte code - either heavily optimized, or perhaps Sierra's parser also supported asm, and these functions were written in asm. At some point, I have to say "I've done enough work on this", and just make sure I can at least detect I wasn't able to decompile it properly, and fall back to asm. At this point, I'd say I have issues with maybe one out of every 20 or 30 functions or so.

Well, ok, here's one example. The method "advance" from SQ5 script 26.

I finally had a chance to look over this. I tried working it out myself by hand. The bits before and after the loop are straightforward I guess, but the loop itself is a bit strange. It kind of looks like a for loop, but if it is a for loop then it doesn't have an end condition check. It feels like some sort of forever loop with a break statement, but there is loop initialization and re-initialization code in there, i.e. the first and third parts of a for loop. It doesn't seem to have the second part though (the condition), so I wondered whether maybe the its a "(for (initialization code) TRUE (re-initialization code)  " or maybe a "(while TRUE " where the initialization code is before the while and the re-initialization code at the end of the code inside the loop. If it was something like this, we'd probably see some sort of evidence for the TRUE check though, unless the optimizer removed it. So maybe they had another type of loop that represented a forever loop and didn't require the TRUE.

The reason that I don't think the code that breaks out of the loop is the second part of the for loop (i.e. the continue looping condition) is because it seems to be in the wrong place. Normally that check would be at the start of a loop, not the end. Unless...,  it is possible that everything within the loop is the conditional check and it doesn't actually have any code inside the loop itself?

I haven't yet come up with a firm idea of what I think it is but those are my initial thoughts.

[Cloudee1]

Troflip, did you happen to address the number of cases a switch statement can have?  I think it topped out at less than 20.  Assembly support would be great too, though I suppose this is a non-trivial feature.

This thread suggests the limit was with SCIStudio, not SCI Companion:
http://sciprogramming.com/community/index.php?topic=588.0

asm support isn't too difficult.

Correct, companion doesn't seem to mind how big a switch is. I have had a few that are very large and haven't had any issue cycling through them or jumping to a specific case. That was an sci studio issue. I just popped open voodoo girl and the intro is sitting at 52 case statements and I don't have an issue with it.

[troflip]

I finally had a chance to look over this. I tried working it out myself by hand.

This is what I get when I work the loop out by hand. Does it look right to you?

Code: [Select]

(while (TRUE)
= temp0 self:at(temp3)
(if (IsObject(temp0) and (not (& (send temp0:signal) 4))
(if ((> (send temp0:nsLeft) -1) or (not (send temp0:isKindOf(InvI))))
break
continue // Impossible to reach
)
)

(= temp3 (% (+ 1 temp3) size))
)

Having the "continue" (which is not supported in SCI Companion or SCI Studio) doesn't really make sense, since it's impossible to reach it.

Another weird thing, which I've noticed occasionally in other scripts, is the "bt code_00cd" branches if true to "bnt code_00d3". The bnt is basically a no-op, since the branch will never be taken. The bt may as well just have branched to the instruction following the bnt. I suspect this is just an artifact of how Sierra compiled compound if statements, and it made no difference in actual functionality, so it wasn't changed.

[lance.ewing]

Yeah, that definitely seems to match, exactly as far as I can tell. What I had was almost the equivalent using a for loop. But your version with the while loop matches it exactly. One thing I wasn't happy with about the for loop was the jmp to the top of the loop without executing the re-initialization code, which wouldn't happen in a for loop (the jmp I'm talking about is the redundant one, but its presence still suggests it isn't a for loop). A continue in a for loop would still execute the re-initialization bit, but not when its a while loop like how you've got it. So that solves that problem. It still feels like we're missing something though. The break followed by a continue suggests that something else is going on. Perhaps the original source had something different, maybe different keywords with slightly different behaviour, some optimisations might also be involved, but what you've got has to be the equivalent. It matches what the byte code is doing. I guess as long as what is generated is equivalent to the byte code, then it would be okay.

The extra jmp in the byte code is also unreachable, so it feels right in a way that the decompiled code also has unreachable code. But maybe the decompiler could drop it out to be cleaner? Without the continue, it is still equivalent. Maybe it could use the presence of it to determine that it isn't a for loop and then it ignores it after that.

Regarding for vs while loops, I noticed from the SQ5 decompiled scripts that they don't have any for loops in them. From what I can see, some of the while loops appear to be for loops in disguise.

Regarding your last point, it seems like there are a number of artifacts like the one you mentioned that the decompiler can probably ignore. I guess it has to know about them but can then ignore them once identified.

[troflip]

The break followed by a continue suggests that something else is going on. Perhaps the original source had something different, maybe different keywords with slightly different behaviour, some optimisations might also be involved, but what you've got has to be the equivalent. It matches what the byte code is doing. I guess as long as what is generated is equivalent to the byte code, then it would be okay.

I agree with you there. Surely this provides some extra clues regarding the original syntax, or maybe just their compiler's optimizations. But in the end, as long as the generated decompiled code is correct, that's good enough.

The extra jmp in the byte code is also unreachable, so it feels right in a way that the decompiled code also has unreachable code. But maybe the decompiler could drop it out to be cleaner? Without the continue, it is still equivalent.

Oh yes, I would drop it out.

Regarding for vs while loops, I noticed from the SQ5 decompiled scripts that they don't have any for loops in them. From what I can see, some of the while loops appear to be for loops in disguise.

A while loop and a for loop are pretty much indistinguishable, so I just turn any loop of that type into a while loop. My decompiler never generates for loops. You could play a guessing game to determine what went in the original init expression and loop expression, but there isn't really much point.

[troflip]

I've made changes so that that method in question can now be decompiled. The result looks like this:

Code: [Select]

    (method (advance param1)
        (var temp0, temp1, temp2, temp3)
        = temp1
            (if (paramTotal)
                param1
            )(else
                1
            )
        = temp3 (% (+ temp1 (= temp2 (self:indexOf(highlightedIcon)))) size)
        (while (TRUE)
            = temp0 (self:at(temp3))
            (if ((IsObject(temp0) and not (& (send temp0:signal) $0004)) and ((> (send temp0:nsLeft) -1) or not (send temp0:isKindOf(InvI))))
                break
            )
            = temp3 (% (+ temp3 1) size)
        )
        (self:highlight(temp0 1))
    )

Ideally, it would be nice to pull the "(= temp2 (self:indexOf(highlightedIcon)))" out of the expression and into a separate line. I think I *might* be able to do that.

Another method in that same script ("retreat", instead of "advance"), looks like it might have a genuine "continue" statement in it (instead of just the unreachable one). It fails to decompile, but I haven't investigated too deeply as to why just yet. The control flow graph for that one is really complicated.

[lance.ewing]

I had a quick look at that script. Glancing over the byte code, it looks like it's function is to do the reverse of the advance method, which makes sense. So given that, I would have thought that the original source for the two methods would be very similar but iterating in the other direction, with temp3 being decremented at the start and on each iteration. A quick scan over it and this seems to be the case. So if the continue seems to be different in this case, it is probably an artifact of some sort and it might shed more light on the structure of the advance method as well.

ScrollableInventory is extending Inv in script 995. The same methods in Inv in script 995 look quite similar in structure. I wonder if it might help to look at that one in conjunction with script 26. Presumably they copied the same source for the method and made a couple of tweaks.

I just had an unrelated thought triggered by the above. Looking at subclass names might be a way of determining the original long names for some classes. For example, ScrollableInventory extends Inv. This to me suggests that the original long name for Inv is Inventory, which is fairly obvious in this case but we might find other examples like this that could shed some light on original class names.

[troflip]

So if the continue seems to be different in this case, it is probably an artifact of some sort and it might shed more light on the structure of the advance method as well.

The difference comes at the end. The code at the end of retreat looks like this:

Code: [Select]

    (if (condition)
        continue
    )
    doSomeStuff
) // end of infinite loop

That could still be rewritten like this:

Code: [Select]

    (if (condition)
    )(else
        doSomeStuff
    )
) // end of infinite loop

So it's not quite absolute proof that the original syntax supported a continue statement, it might just have been a compiler "optimization" that produced this code.

[troflip]

Today I compiled my first SCI1 script (SQ5). I am just focusing on recompiling the decompiled version of script 100. After checking that the script and heap resources looked fairly consistent with the original, and looked normal in SV.exe, I tried running the game.

It crashed in both Sierra's interpreter and ScummVM. Luckily with Scumm's debugging console I was able to track down the problem (see my other post in the LEA thread), and made a fix and it then worked in Scumm.

Unfortunately, it still crashes the Sierra interpreter. So *something* is different... *somewhere*.