Opened 18 years ago
Closed 8 years ago
#2687 closed defect (fixed)
BASS: Officer Blunt wrong animation (?)
| Reported by: | SF/adventureguy | Owned by: | digitall |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Sky |
| Version: | Keywords: | ||
| Cc: | Game: | Beneath a Steel Sky |
Description
Hello! I think I found a bug in the game Beneath a Steel Sky. I tested the English Talkie game on Windows with the ScummVM stable 0.8.2 built and the 0.9.0pre built on 17th June 2006. When I try to talk or give something to Officer Blunt on the ground level after the dog is swimming in the lake, he talks in his normal speech animation, so it looks like he's standing in the lake. I attached a screenshot. Was this also a problem in the original game or is this a bug in ScummVM?
Ticket imported from: #1507756. Ticket imported from: bugs/2687.
Attachments (8)
Change History (23)
by , 18 years ago
comment:1 by , 18 years ago
| Summary: | Officer Blunt wrong animation (?) → BASS: Officer Blunt wrong animation (?) |
|---|
by , 18 years ago
| Attachment: | SKY-VM.SAV added |
|---|
Might be needed for loading savgame. It's savegame 5.
comment:2 by , 18 years ago
I now added a savegame. Just give Blunt any item, he will act strangely (at least for me).
comment:3 by , 18 years ago
"Was this also a problem in the original game or is this a bug in ScummVM?" - it would be great if someone could confirm/deny this problem also exists in the original.
comment:5 by , 14 years ago
Replicated with latest bass-cd-1.2.zip i.e. "Beneath a Steel Sky (v0.0372 cd)" on : ScummVM 1.2.0svn52433 (Aug 29 2010 03:40:31) Features compiled in: Vorbis FLAC MP3 ALSA SEQ RGB zLib FluidSynth on Linux x86_32 2.4.34
Will try to replicate with original interpreter.
comment:6 by , 14 years ago
Failed to replicate with original interpreter 0.0372 CD under Dosbox. This looks like a real regression. Attaching savegames and original interpreter executables to allow replication of failure to replicate.
by , 14 years ago
| Attachment: | original-0372-interpreter.zip added |
|---|
Original 0.0372 Interpreter to show failure to replicate
by , 14 years ago
| Attachment: | original-0372-savegames.zip added |
|---|
Original 0.0372 Savegames to show failure to replicate
comment:7 by , 14 years ago
Played through to this point with ScummVM 0.7.0 prebuilt win32 and same game data and bug is not present Tried the savegame produced on v1.1.1 prebuilt win32 and bug is present.
Determined that v0.7.1 does not exhibit the bug, but v0.8.0 does. This indicates that the regression is between r17267 and r19355.
Will bisect between these to narrow the cause.
comment:8 by , 14 years ago
Ah, I had neglected to realise that those revisions are correct, but the code is from the release branches, not the trunk.
0.7.1 was derived from 0.7.0 which forked from the trunk at r16049 and 0.8.0 was forked at r19131, so the correct range for the cause on the trunk was between r16049 and r19131.
Have traced back with builds to r17116 on trunk with the bug still occuring, but tracing earlier is presenting some difficulties with newer GCC, but the most likely hypothesis is as follows:
That the bug is not occuring in 0.7.* as these versions do not use the external compact file, SKY.CPT, instead using data built into the executable. This was changed in trunk revision r16053 which falls in the range above, so that 0.8.0 and later builds which use the external SKY.CPT file exhibit the bug.
comment:9 by , 14 years ago
I have now managed to builds earlier than r17116 and can confirm that this hypothesis is confirmed.
The last build without this issue is r16052. r16053 then moved the data out to the external sky.cpt file : ------------------------------------------------------------------------ r16053 | lavosspawn | 2004-12-15 06:48:08 +0000 (Wed, 15 Dec 2004) | 3 lines
moved compacts and related static data out of the scummvm.exe into an external file (available from www.lavosspawn.de/tmp/sky_cpt.zip) ------------------------------------------------------------------------
After this, there are a number of revisions with broken builds, followed by a lack of animation altogether.
This was corrected by r16828, which is also the first revision showing the issue with Blunt i.e. : ------------------------------------------------------------------------ r16828 | lavosspawn | 2005-02-20 18:33:36 +0000 (Sun, 20 Feb 2005) | 2 lines
fix missing speech animations. I introduced that bug when I moved the compacts out of the scummvm executable. ------------------------------------------------------------------------
Am attaching build script and a couple of minor patches, which are necessary (at least for me) to build revisions of this age under newer GCC (4.4.3 in my case), for reference.
Will now start looking for the exact difference in the engine / compact data which is causing this.
by , 14 years ago
| Attachment: | build-scripts-for-gcc-4.tar.gz added |
|---|
Build Scripts to build ScummVM ~r16000-17000 with GCC-4
comment:10 by , 14 years ago
Have traced the fault by adding debugging output to code in sky/logic.cpp for both r16052 and r16074 (with r16828 patch applied) i.e. without and with bug.
The following output occurs while speaking to Officer Blunt while crouched : Logic::fnSpeakWait(id=16601, message=17168, animation=158) Logic::fnSpeakMe(targetId=16601, mesgNum=17168, animNum=158) Logic::stdSpeak(textNum=17168, animNum=158, base=0)
However, Animation 158 maps to target->grafixProgId = 16696 in the correct case and 16607 in the buggy case.
Looking these up in sky/skydefs.h shows that these are: ID_SC31_GUARD_TALK (standing) 16607 ID_SC31_GUARD2_TALK (crouching) 16696
In the older correct code, sky/talks.h holds a table animTalkTableVal[] mapping these and offset 158 in this table is the correct ID_SC31_GUARD2_TALK, but the newer code returns ID_SC31_GUARD_TALK instead from the external compact sky.cpt. By tracing the code in compact.cpp, I have determined that the errorneous value is present in the SKY.CPT file at byte offset 0x42F6. Changing this to16696 corrects the issue.
Attaching a corrected CPT file.
If this is not a desirable solution, this fix could also be accomplished by adding a code workaround to sky/logic.cpp to check for Animation 158 and force 16696.
comment:11 by , 11 years ago
As this bug has not been updated for some time, I am adding this comment to record why and the current status.
We now have a solution which fixes this bug i.e. a binary patch to the SKY.CPT external data file and/or a code workaround to the sky engine which overrides the grafixProgId value for Animation 158 and "forces" / uses a hard coded value.
However, this has not been committed and the bug closed as this is not an acceptable solution to several team members. The reason for this is as follows:
We have the source code for the tool to generate sky.cpt in the tree as devtools/skycpt, but this was not designed for general use and was only committed to comply with Debian licensing as this has issues with "binary blobs" and requires the source code of any tool used to generate them: https://github.com/scummvm/scummvm/blob/master/devtools/skycpt/README#L13
This tool uses savegame states from all the game versions saved prior to the removal of the static data from the sky engine as input data to generate the sky.cpt file. As such, it is not easy to modify and without knowing the full format of the external SKY.CPT datafile, any further impact of the binary patch proposed is unclear i.e. it may solve this bug and cause others :/
Thus for this bug to be closed, we need to either patch or replace the skycpt tool to ensure that the Debian licensing issues are satisfied i.e. that the binary external datafile, sky.cpt is accurately and reliably produced from an open source tool.
comment:12 by , 9 years ago
Would it help if, in addition to the tool, we also had these save games archived somewhere? I don't have the necessary versions of the game, so I couldn't provide them even if I wanted to, so I'm just asking out of curiosity.
comment:13 by , 8 years ago
Fixes for this bug have now been committed to the master tree by lavosspawn as three commits: https://github.com/scummvm/scummvm/commit/7874f517f79da796e480e014d72f536d3abe2af8
https://github.com/scummvm/scummvm/commit/e40b4850f7ac94766c90873a9ac3e598f31bba40
https://github.com/scummvm/scummvm/commit/4d94bdc0648f19b41be6d6bc313244165a3448a5
Marked bug as fixed.
comment:14 by , 8 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → pending |
comment:15 by , 8 years ago
| Owner: | set to |
|---|---|
| Status: | pending → closed |
Blunt's animation