#9202 closed patch
descumm: better string output and semicolon cleanup for
| Reported by: | SF/jestar_jokin | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Tools |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
This patch comprises of fixes for two issues:
1) descumm is outputting semicolons in odd places for SCUMM <= V5 scripts. - doSentence is missing semicolons when outputting special values "STOP" and "RESET" - setVarRange is outputting semicolons in the middle of the instruction - drawBox is outputting semicolons in the middle of the instruction
2) Improved and standardised string output. The string output method has been consolidated into descumm-common.cpp, and is now shared between all SCUMM versions (except HE games, and possibly low SCUMM versions, but tested against V5 and V6). In addition, extra information is now output for some in-line string functions, and the formatting of the string functions and how they join with the rest of the string has been changed. For example, an old string might look like this:
talkActor(:sound:"It makes me feel GREAT!":wait:"Smarter! More aggressive!",7)
new version:
talkActor(sound(0x3BD5, 0x16) + "It makes me feel GREAT!" + wait() + "Smarter! More aggressive!",7)
HE string output has not been changed. I've added an extra workaround for Indy3 scripts (gleaned from the main ScummVM source).
Ticket imported from: #3049421. Ticket imported from: patches/1307.
Attachments (3)
Change History (14)
by , 14 years ago
| Attachment: | string_output_and_semicolons.diff added |
|---|
comment:1 by , 13 years ago
Is there anything stopping this patch from being looked at? I realize that the new generic bytecode decompiler will replace descumm, but it's not currently in a useable state, hence the need for updating descumm.
Would it be better if I separate this into two patches, one for the "semicolon" fix and one for the "shared string output code"?
If this will not be incorporated into ScummVM, please let me know.
comment:3 by , 13 years ago
You are absolutely right on all accounts. Please split the patch -- I recently actually wanted to just apply this patch between two other obligations, thinking it was a trivial semicolon fix. Then I realized that it moved quite some code around, and decided that I better not rush this. Breaking it down should make it easier to apply it. I'll be happy to review it and apply it, too.
comment:4 by , 13 years ago
| Owner: | set to |
|---|
by , 13 years ago
| Attachment: | semicolons.diff added |
|---|
just semicolon fix, patched against svn54905, generated with TortoiseSVN
comment:5 by , 13 years ago
I have attached "semicolons.diff", which should be all the changes just for fixing the semicolons.
For generating the "stinrg output" diff, should I diff against what's in SVN (before the semicolons change has been applied), or my working copy (after the semicolons change has been applied)?
comment:6 by , 13 years ago
I commited the semicolon patch, so the rest should be against that. Thanks!
by , 13 years ago
| Attachment: | common_string_output.diff added |
|---|
common string output changes, patched against svn54931, generated with TortoiseSVN
comment:7 by , 13 years ago
I have uploaded the page for the "common string output". Let me know if you have any questions regarding it.
comment:8 by , 13 years ago
I thought I should summarise what's contained in the changes.
- Content of "se_get_string" for V6+ moved in to descumm-common. - Separate support for retrieving variable names in the string for <= V5 and >= V6. This includes adding the function "char get_var6(char *buf)" - Remove "get_ascii" function for <= V5, since it did not know anything about the in-line string functions. Replaced with "get_string" in descumm-common. - The common "get_string" will make use of "put_ascii" (copied from <= V5), so any non-function characters will get escaped as appropriate (e.g. quote marks, backslashes). - Rename some of the in-line string functions. e.g. "addIntToStack" is now "getInt", since <= V5 does not use the stack. (perhaps "insertInt" would be more descriptive?). Also, "unknown2" renamed to "unknown13", which matches the naming of all other unknown functions. - The "sound" in-line string function now outputs all its parameters - Handles odd cases for script bugs in Indy3. (perhaps it's better to show that there is a script bug?) - In-line string functions format has changed. Now appears like regular function calls (e.g. 'sound(0x3BD5, 0x16') instead of ':sound:'), and each function is concatenated with the text of the string (e.g. '"Hello there!" + wait() + "It sure its a beautiful day!")
comment:9 by , 13 years ago
| Status: | new → closed |
|---|
comment:10 by , 13 years ago
Actually, the "stack" in addStringToStack has nothing to do with the stack used by v6+ (nor with the expression stack already used in v5). Rather, the name was copied from old versions of ScummVM; in the ScummVM source, this function is now called convertStringMessage. The "stack" part in the function was due to the way this function was implemented.
Nevertheless, this name was internal only, and it should not be exposed in output of descumm, so printing it as "getString(var123)" is definitely better than using that name. In fact, I would also suggest replacing the reference in the comment to addStringToStack by convertStringMessage -- this is helpful for people who need to match code in descumm and the ScummVM code base.
Anyway, back to your patch. To get it to compile, I had to add declarations for put_ascii and get_string to descumm.h, and also declare get_var and get_var6 as extern functions in/before the code for get_string. Afterwards it worked. And it's nice to get translated versions of the special codes in V0-V5 text!
However, I am not sure whether I like the way the output is formatted, using a string concatenation style. This makes some sense for the getString(var123) case above. But for e.g. wait(), it doesn't really seem too good a match. It's also relatively verbose, comparing "foo :wait: bar" to "foo" + wait() + "bar" On the other hand, it makes it easy to avoid escape handling...
All in all, I still don't like it very much, but since it is a lot better than what we have now, I am going to commit it anyway; it'll be easy enough to change the output to something else should we desire so in the future. (Though I hope we'll replace this by the new decompiler code instead).
comment:11 by , 5 years ago
| Component: | → Tools |
|---|
patch against svn52202, generated with TortoiseSVN