QFG1VGA: 203 - recompile after decompile fails about scicompanion

[Kawa-oneechan]

Okay. So the plan:

• You fix it yourself either by changing dsCOORD back to 100, or adding the extra parameter, whenever a (Display &rest ...) appears. Your choice.
• I will look into fixing the compiler, whenever, so &rest knows what to do when followed by a constant.

[Kawa-oneechan]

Hmm. Apparently my Display massager doesn't play nice with whatever lines you're referring to.

I'll look into it tomorrow but next time please include the actual lines themselves instead if only the error messages.

[Kawa-oneechan]

Okay, so what's happening is, my fork can detect Display calls and automatically replaces 100 with dsCOORD and all that. The context you left out reveals this is a wrapper function to draw double-struck text:

(Display &rest dsCOORD 220 param1 dsCOLOR 215 dsWIDTH 30 dsALIGN 1 dsFONT 123)

And then the compiler gets confused about the fact that dsCOORD is not a number or a variable of some kind, but a defined constant value.

In original Sierra parlance, it'd be written like this:

(Display &rest #p_at: 220 param1 #p_color: 215 #p_width: 30 #p_mode: 1 #p_font: 123)

And the SC compiler would know from the # and : what it was looking at.

One fix on your side that wouldn't even require changing any calls to localproc_095b would be to give it a second parameter:

(procedure (localproc_095b param1 param2)
	(Display &rest param2 dsCOORD 220 param1 dsCOLOR 215 dsWIDTH 30 dsALIGN 1 dsFONT 123)
	(Display &rest param2 dsCOORD 219 param1 dsCOLOR 91 dsWIDTH 30 dsALIGN 1 dsFONT 123)
)

That is, give &rest a parameter name to work with, removing the ambiguity on what dsCOORD is doing there. With properly named arguments, this would be a very readable result.

A much more difficult fix on my end would be to detect constants following &rest keywords so it knows to handle them like the numbers they are. And frankly, I have no intention to do this and would rather throw this into the same small pile as the one where a decompiled statement involving a freshly-instantiated object that's immediately used really ought to have a temporary variable for that object.

[ZvikaZ]

Another possibility is to add a comment, explaining that a numerical value was replaced by a constant. Thus, if there are compilation problems (in similar occasions in the future), the user will know that he might try replacing it back to that specific numerical value.

e.g.,

; Kawa says that dsCOORD == 100
(Display &rest dsCOORD 220 param1 dsCOLOR 215 dsWIDTH 30 dsALIGN 1 dsFONT 123)

(of course, the message format is just a suggestion ;-) )

[Kawa-oneechan]

It's not me saying that. The ds_____ constants are in sci.h, specifically meant for use with Display, so it's really Phil and Brian saying that.

Also, just adding that comment will not actually fix anything. You still have &rest dsCOORD there, so it'd still not compile. Manually adding a second parameter after decompiling is the easiest fix and helps make the function's parameters more clear.

[ZvikaZ]

Quoting you was just a joke...

I'm not saying that it will fix the problem. I'm merely saying that if the decompiled code has a change of not going to compile, it'd be nice to give a comment that might give hint for what's the direction to go to solve it.

IMO mentioning the original value of the constant seems to be the easiest solution, both for you, and for the user.
But if you think that some other solution is better - it's OK with me...

The core of my idea is to add a comment, that will give the user a clue when something is going wrong to what he can do to manually fix it.

[Kawa-oneechan]

It doesn't matter if you quote me or not, just putting a comment explaining a change does not actually make the change. It's effectively not a solution at all.

If you meant that the decompiler should insert such a comment, that'd mean detecting the potential problem, and you might as well actually do the in-compiler fix that I listed last.

[ZvikaZ]

I totally agree with your first part :-)

I'm indeed talking about the decompiler adding such a comment. And since recognizing the situation is complicated, I don't want it to add the comment just in the problematic case; I want it to always add such a comment.

Let me rephrase my suggestion. In Phil's version, the decompiled code compiled without problems.
In the updated code, there is a new cool feature - replace the numerical value with a constant name.
That's a good feature - but it has a risk - sometimes the decompiled code won't compile again.
Making a good fix for that it currently out of scope (if I understand correctly). So, I suggest a small change, that won't fix anything, but will only make it easier on further occurrences of the problem to manually fix it - just notify the user that there was here a numerical value that was replaced, and if needed, the user would be able to easily take the numerical value from that comment.

[Kawa-oneechan]

So you're saying that every Display that the decompiler emits should mention that it uses constants to specify subcommands?

It's not that fixing it is out of scope, be it having the decompiler recognize there's a (Display &rest ...) (noting that it's almost useless without any subcommands) and adding a new parameter itself, or having the compiler recognize constants after &rest for what they are. It's that so many kernel functions in SCI use subcommands with define or enum constants, it would add nothing but noise. Making a good fix is perfectly in scope, it's just quite difficult because have you seen that compiler?

Frankly I would rather add more constants, so for example an "open file" operation would be (FileIO fiOPEN "lol.txt") instead of having a number there. I have some actual experience with that now, and the only difficulty would be different SCI versions having different subcommands for certain kernel functions.

[ZvikaZ]

OK, you've convinced me. :-)
Don't add that comment...

Hopefully, you'll succeed in making a real fix for the problem.

Anyway, that's a good opportunity to thank you for maintaining SciCompanion and the continuous thinking how to improve it.