Kawa-oneechan / SCICompanion

SCI Companion - an editor for Sierra SCI games, altered.
18 stars 7 forks source link

QFG4CD: Fix parsing/compiling escaped backslash in string literals #13

Closed laurence-myers closed 1 year ago

laurence-myers commented 1 year ago

Parser fix

ParserCommon.h: _ReadStringSCI: fix parsing \\

In QFG4CD, script 54 "Import" tries to add a backslash to a string, using the syntax {\\}. The parser used a naive test to see if the closing quote char was escaped, by checking if the previous character was a backslash, which falls over when that backslash is escaped by another backslash. I've fixed it by instead checking the "fEscape" boolean.

I've also added an explicit case for parsing an escaped backslash. This might fix/impact the "default else" case - we may want to uncomment the statement processCharNormally = true, since my changes might fix the case for sciAudio\\command.con.

Syntax highlighting fix

ScriptView.cpp: fix syntax highlighting for double-quoted string literals containing double-backslashes (escape characters), in SCI script syntax.

Additional changes

Testing

Before

2023-03-28 14_11_37-SCICompanion -  MyButton_54 sc

2023-03-28 14_11_59-SCICompanion -  MyButton_54 sc

After

As before, but this time the syntax highlighting is constrained to the actual string literal, and compilation succeeds.

2023-03-28 14_20_31-SCICompanion -  MyButton_54 sc

2023-03-28 14_20_49-SCICompanion -  MyButton_54 sc

Other notes

In ParserCommon.h, _ReadStringSCI, there's a line of code commented out: processCharNormally = true. The comment near it mentions that it was causing problems parsing sciAudio\\command.con into sciAudio\\mmand.con. I think my changes fix the parsing of the double backslash, so we might be able to restore the commented code. I don't know what game/script it's referring to, so I can't test it myself.

(However, it might be even better to just fail parsing, if we find an invalid escape sequence like \c. I don't know if that's possible currently.)

Caveats

Kawa-oneechan commented 1 year ago

Regarding your other notes, that whole thing refers to SCI0 fan games writing instructions to a file for an external program to pick up that plays better soundtracks. All you should need to test it is to have a string literal with that path in it, see if it compiles and decompiles correctly. You do that, and I'll be willing to hit the merge button.

As for your caveat, that's actually not specific to C or C++ but rather Win32.

laurence-myers commented 1 year ago

All you should need to test it is to have a string literal with that path in it, see if it compiles and decompiles correctly. You do that, and I'll be willing to hit the merge button.

Great! I tried it out, and it looks good to me. I changed the string literal to {sciAudio\\command.con}, compiled, and decompiled, and it preserved the string. πŸ™‚

Initial change & compile: 2023-03-29 21_56_30-SCICompanion -  MyButton_54 sc

Decompile: 2023-03-29 21_57_02-Decompile

Decompiled script: 2023-03-29 21_59_50-SCICompanion -  MyButton_54 sc

I then tried changing that last "else" condition. Now, if you pass an invalid escape sequence, like \c, it will output just c. I think this is less surprising than outputting an extra backslash. I've pushed up another commit with this change.

Initial script: 2023-03-29 22_21_59-SCICompanion -  MyButton_54 sc _

After decompile: 2023-03-29 22_22_28-SCICompanion -  MyButton_54 sc