#6379 closed defect (fixed)
TUCKER: Spanish version uncompletable at end of 2nd part
| Reported by: | (none) | Owned by: | digitall |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Tucker |
| Version: | Keywords: | ||
| Cc: | Game: | Bud Tucker |
Description
In the second part of the game, when you have donde all the things and have the three items to escape: - a false nose - the cassette player with the recorded voice of the sleeping guard - the security pass
if you go to the room where the professor is, Bud talks automatically with him and the second part ends.
In ScummVM, Bud doesn't talk automatically with the professor, and if you talk to him (manually), you have a conversation, and then you return to control, not completing the second part. As i told, in Dosbox, Bud speaks automatically with the professor and the second part ends.
Please, fix it, makes the second part of the game never be completable.
Savegames from Dosbox and ScummVM attached. Load they and go to the store room where professor is (clicking in the center of the air vent).
Ticket imported from: #3614697. Ticket imported from: bugs/6379.
Attachments (3)
Change History (26)
by , 11 years ago
| Attachment: | ScummVM_second_part_never_ends_bug.zip added |
|---|
comment:1 by , 11 years ago
| Component: | → Engine: Tucker |
|---|---|
| Game: | → Bud Tucker |
| Priority: | normal → blocker |
by , 11 years ago
| Attachment: | Dosbox_savegame_second_part_ends_correctly.zip added |
|---|
Dosbox savegame to see the correct behaviour (second part ends when you go to the store room)
comment:2 by , 11 years ago
| Priority: | blocker → normal |
|---|
comment:3 by , 11 years ago
Both savegames are identical, same objects, same position in the inventory, etc.
I think the bug can be that ScummVM didn't correctly set a flag during my gameplay when I have taken some of the three objects, or when i done some uses with they, so when you go to the store room and i have done all, the engine checks those flags, and if they are not correctly setted, the game believes that you must do more things and the second part never ends...
comment:4 by , 11 years ago
I have discovered the problem. I will give full detail to help developers. It is very easy to fix.
THE PROBLEM DOESN'T HAPPEN IN THE ENGLISH VERSION. THE PROBLEM IS IN THE SPANISH VERSION (and probably in the other languages, i haven't checked they).
OK, i give full detail:
The game uses the file CSDATA.ENC to read instruccions on it and execute they. That file is encrypted, once decrypted we can see as plain text all the instructions. These instructions are strings of three letters, like: "opt","bus","pan","bua" and some numbers then.
The problem is, that when the game was translated to spanish, they modify the CSDATA.ENC and they put a wrong instruction named "mof". That's why if you execute ScummVM with debug mode (parameter -d11) you can see in the debug window the problem: executeTableInstruction() instruction mof WARNING: Unhandled instruction 'mof'
That works in dosbox because the original code (the tucker.exe) simply skips the wrong instructions and continue executing the following instruction codes, but ScummVM not! This can be fixed only making ScummVM continue executing instructions if a wrong instruction is read.
This is the line from the decoded CSDATA.ENC read when we go to the store room in the original version (english): 61dw buw,148,125,wsm,buw,148,132,wsm,wat,050,ssp,200,040,tpo,0086,bsd,02,bus,112,wa+,c0s,113,wa+,bus,114,wa+,c0s,115,was,c0s,116,wa+,sse,062
All those instructions are the sound to be played, the texts to be shown, etc.
And this, the same line but from the decoded spanish CSDATA.ENC: 61dw buw,148,125,wsm,buw,148,132,wsm,mof,pan,01,wat,050,ssp,200,040,tpo,0086,bsd,02,bus,112,wa+,c0s,113,wa+,bus,114,wa+,c0s,115,was,c0s,116,wa+,sse,062
You can see the "mof" instruction. Also, the spanish version adds the intructions "pan,01" just after the "mof" instruction but these are correct.
The code to modify is in the file: /engines/tucker/tucker.cpp
I think the function that need to be changed is readTableInstructionCode, but i am not sure. At the end of that function you will see:
warning("Unhandled instruction '%c%c%c'", _tableInstructionsPtr[0], _tableInstructionsPtr[1], _tableInstructionsPtr[2]); return kCode_invalid;
I think we need to replace it with that (but i'm not completely sure!):
warning("Unhandled instruction '%c%c%c'", _tableInstructionsPtr[0], _tableInstructionsPtr[1], _tableInstructionsPtr[2]); _tableInstructionsPtr += nameLen + 1; return kCode_invalid;
CAN ANY DEVELOPER APPLY THE FIX?
by , 11 years ago
| Attachment: | Spanish_CSDATA_with_mof_instruction.zip added |
|---|
Replace the original CSDATA.ENC file (english version) with this one (this is the spanish version) to reproduce the error
comment:5 by , 11 years ago
It is not mandatory to have the spanish version to reproduce the error. It can be reproduced that way: replace the CSDATA.ENC file i attached from the spanish version to your english version. You will see the problem
comment:6 by , 11 years ago
| Summary: | TUCKER: VERY IMPORTANT BUG: Makes game be unfinishable → TUCKER: Spanish version uncompletable at end of 2nd part |
|---|
comment:7 by , 11 years ago
| Owner: | set to |
|---|
comment:8 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → pending |
comment:9 by , 11 years ago
Unamed Google Account: Thank you for the bug report. I have reviewed and implemented your suggested change in commit 485118ecd9640538337672e3a00bb312803da666 along with a fix to deal with handling the "mof" instruction in commit 1946dead77378bbf16cb1ed65c9937226c749a5e. This is currently handled as nop, but support has been added along with a TODO as it may have a script function.
Please can you download and try the nightly build tomorrow (when these fixes will be included) and see if this is now fixed for you? http://buildbot.scummvm.org/builds.html
Thanks.
comment:10 by , 11 years ago
| Status: | pending → new |
|---|
comment:11 by , 11 years ago
Sure tdhs, tomorrow i will try the daily build to see if it fix the problem, thanks for the fix!!
I have take a look at your code change, and looks pretty well, you handle the "mof" instruction (although it does nothing as we think) and you handle also any other invalid instruction that can come, to continue executing instructions, perfect!
I tell you a test i did trying to know what "mof" instruction does using the dosbox savegame: - If you use the SPANISH CSDATA.ENC, running the game with the ENGLISH tucker.exe, the game says Invalid instruction "mof" But the spanish tucker.exe accepts it and it works. Does it really the "mof" instruction does anything using the spanish tucker.exe?? I think it doesn't, because i have tried also deleting the "mof" instruction with the spanish exe, and game continues normally... - Actually, the spanish exe doesn't skip the "mof" instruction, it reads it without error, because if you modify the CSDATA.ENC, and you change "mof" with for example "kxx", the game shows: Invalid instruction "kxx". So the "mof" instruction is VALID for the spanish version... Weird!
I will leave a comment here tomorrow telling if the fix works, thanks again tdhs
comment:12 by , 11 years ago
| Owner: | removed |
|---|
comment:14 by , 11 years ago
Sorry tdhs, your fix doesn't work :-(
I executed the ScummVM daily build with the parameter -d11, and i can see: executeTableInstruction() instruction mof
As the 1.6.0. And now the line: WARNING: Unhandled instruction 'mof'
It Is not shown (because your code change handles it), but Bud and the Professor doesn't start the automatic conversation to end the second part... :-(
I think I know where the problem is. You added:
case kCode_mof: // TODO: Unknown opcode in Spanish version. Identify if this has any function. return 2;
But returning 2 is what the "end" instruction does to stop executing more instructions (that is the only instruction that returns 2 in the executeTableInstruction function):
case kCode_end: return 2;
So you should return a 0 or 1 in the case kCode_mof, instead of returning 2.
comment:15 by , 11 years ago
| Resolution: | fixed |
|---|
comment:16 by , 11 years ago
Have amended to 0 as the most likely value in commit 7381fcdf305315591fd2d9c66d9804ceb6fd3996.
Unamed Google User: Please try the next nightly build to confirm this now works.
These return values should be symbols, rather than values as their meaning is unclear. Will amend the source code to improve the readability.
comment:17 by , 11 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → pending |
comment:18 by , 11 years ago
Now it works! Using the spanish Bud Tucker, it reads the "mof" instruction and the game continues successfully, and the third part begins (after the long conversation). Thank you tdhs.
But we need one more change... Reading an unknown instruction doesn't work (as you remember i suggested you to also modify the code to continue execution when a unknown instruction is readed to bypass other possibles bad instructions). I have tested it changing "mof" to "kxx" to force the engine to read one of these. We missed a thing... we must return 0, instead of 2 (again), when an unknown instruction is readed, so, i think the change is easy. In executeTableInstruction function, change the final return:
So, this:
... case kCode_xhm: _validInstructionId = false; return 0; } return 2;
By this: ... case kCode_xhm: _validInstructionId = false; return 0; } return 0; //we return a 0 to continue executing instructions
comment:19 by , 11 years ago
| Status: | pending → new |
|---|
comment:20 by , 11 years ago
| Status: | new → closed |
|---|
comment:21 by , 11 years ago
Unamed Google User: As the original bug is now fixed, I am going to close this bug.
As for the change you suggest, I have thought about this for quite some time and on balance, I am going to leave the code as-is. There is always an argument between hiding or coping with unknown opcode/datafile values or failing quickly and noisily so they get fixed. As we have no idea of the meaning of an unknown opcode, we have no idea how to process it correctly, so I am not going to change the original engine authors decision to cause scripts to halt on unknown opcodes. A warning is emitted in this situation and this should be sufficient to flag any other cases for fixing on a few basic playtests.
comment:22 by , 11 years ago
Another argument here is that causing scripts to continue on unknown opcodes could expose as many latent bugs as this solves, so it is not a good solution.
comment:23 by , 6 years ago
I looked at the disassembly for the Spanish version and what the 'mof' opcode actually does is simply disable the mouse (mof - mouse off).
I have now fixed this in commit 23fd97c99a650cf2e99df72a74326520b9347fc7.
As an aside, the Polish version has an identical action in CSDATA.C and thus also uses 'mof'.
ScummVM savegame to reproduce the error