Opened 6 years ago

Closed 6 years ago

#10861 closed defect (fixed)

COMPOSER: Darby the Dragon: ERROR: Archive does not contain 'ANIM' 2329!

Reported by: Rockin2 Owned by: bluegr
Priority: blocker Component: Engine: Composer
Version: Keywords: regression
Cc: Game:

Description

When I begin the game, I go to the throne room and I click the magic wand. The game crashes with this error.

With the original engine, there is Darby that say we don't need to use the magic wand here.

Attachments (1)

Darby the dragon error.png (163.6 KB ) - added by Rockin2 6 years ago.

Download all attachments as: .zip

Change History (14)

by Rockin2, 6 years ago

Attachment: Darby the dragon error.png added

comment:1 by digitall, 6 years ago

I can replicate this with the latest git master on Linux x86_64 with the English CD version of Darby The Dragon, so unlikely to be damaged datafiles.

comment:2 by digitall, 6 years ago

Aha. This is a regression. I tested commit 907c053fcab39b0601d9f92b33d994a4503de9a6 (Sat Dec 7 00:05:45 2013) and this works fine, so will do a bisection and locate the regression point.

comment:3 by digitall, 6 years ago

Right. This problem was introduced by the merge of PR 851:
https://github.com/scummvm/scummvm/pull/851

It is not clear which commit in that PR is causing this as none of the commits build :/

The breakage is somewhere between 1efb11818d8373f0028de575596a484d8dcb3b44 which is good and ec06c04faa7a14e3586849a670ff8db71d43a7ca which is bad, but all intermediate commits fail to build.

Will try to apply later fixes to the relevant commits to try to locate the cause of this.

comment:4 by digitall, 6 years ago

OK. Have created a test branch based on the centre point of this broken region i.e. 69a6a200a21083b1ee7970d82fad0384a64bdcef and then applied the four commits from angstsmurf which correct compilation i.e. e9d3766f75bf470d7aa7e413478d8a2bdfb7dec0, d526e9969ce0e66a6b51a3374af9dd8ef1d63d34, c50c35066430ab282df36deeba3416c89e274de6 and ad41322b64d54f608d5ddaf6ddf1fd39cd9c9679.

The resulting code compiles correctly and still exhibits the bug so it is introduced between 1efb11818d8373f0028de575596a484d8dcb3b44 which is good and 69a6a200a21083b1ee7970d82fad0384a64bdcef which is bad which narrows the range slightly.

Will repeat this again to see if I can get this down to a specific commit and code change.

Version 0, edited 6 years ago by digitall (next)

comment:5 by digitall, 6 years ago

Right have managed to get c4c6cce78ea67f26b1c2017ebb15f00612c4d41b to build with minor corrections for setPixels() and getPixels() in saveload.cpp and some commented out code which will not break any normal gameplay (but would break save/load). This does not exhibit the bug so it is between c4c6cce78ea67f26b1c2017ebb15f00612c4d41b which is good and 69a6a200a21083b1ee7970d82fad0384a64bdcef which is bad.

The cause is likely to be commit d91368aa1a2f7b4bc7b270cfbd72db9c97b9441f since of the two commits which could be the first bad one, this one touches the engines/composer/graphics.cpp and makes extensive changes to void ComposerEngine::playAnimation and related code.

comment:6 by digitall, 6 years ago

Right. To investigate this, you will need to make a test branch based on 69a6a200a21083b1ee7970d82fad0384a64bdcef and then applied the four commits from angstsmurf which correct compilation i.e. a946e9eab95201d4ee3dd0d4eaeb3a088fb819fe, 17f3626214d3244bfe07b4f263c99be2de07a289, c3994cd6ea1a616164869f54659339b462aafc2a and ec06c04faa7a14e3586849a670ff8db71d43a7ca.

The regression is due to the changes in d91368aa1a2f7b4bc7b270cfbd72db9c97b9441f, but this requires the next two commits which correct issues preventing compilation which results in commit 69a6a200a21083b1ee7970d82fad0384a64bdcef...

comment:7 by digitall, 6 years ago

Another way of looking at this was to trace down where the error() call is made from. This is from the engines/composer/resource.cpp code. However, since the same error text can be generated in about 10 locations, I had to modify the error strings with a number to determine which was the triggered one.

This was from line 91 i.e. Common::SeekableReadStream *Archive::getResource(uint32 tag, uint16 id) function when it checks for the id in the resMap ...

comment:8 by digitall, 6 years ago

Have also pushed my bug investigation branch in case anyone needs it (it is as per comment:6):
https://github.com/digitall/scummvm/tree/bug10861

comment:9 by digitall, 6 years ago

Aha. I think I can see the cause of the issue, though not clear what exactly the fix code should be.

The graphics.cpp changes in d91368aa1a2f7b4bc7b270cfbd72db9c97b9441f now try to check for resources in all of the _libraries archives by calling getResource() and then testing if the resulting stream is NULL i.e.
https://github.com/scummvm/scummvm/blob/master/engines/composer/graphics.cpp#L110

However, the getResource() call will make a fatal error() call aborting the engine if you request a non-existent id i.e.
https://github.com/scummvm/scummvm/blob/master/engines/composer/resource.cpp#L85

The solution should be to replace this with hasResource() calls which are not fatal and only when the resource is located, then do a getResource() to get it's data...

comment:10 by Rockin2, 6 years ago

Sorry! The email notifications was in my spam box. I think you have the solution to fix the bug. However, there is something funny with Darby the Dragon. If I remember correctly, there is a text file that lists all resources files. On all Darby the Dragon copies, there is a missing resource file listed by it. It's like there was a bug (or missing resource) in the game but the original game engine just bypass it. :p

comment:11 by bonki, 6 years ago

Keywords: regression added; Darby Composer removed
Summary: Darby the dragon: ERROR: Archive does not contain 'ANIM' 2329!COMPOSER: Darby the Dragon: ERROR: Archive does not contain 'ANIM' 2329!

comment:12 by Filippos Karapetis <bluegr@…>, 6 years ago

In e45dd706:

COMPOSER: Always check resources before loading them

Fixes checking for resources in libraries - bug #10861

comment:13 by bluegr, 6 years ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed

Thanks to digitall's work, this was easy to fix. This was the only call where hasResource() was not called before getResource().

Adding the check, like in the other cases, fixes this error. I now get the expected behavior when using the wand in the throne room.

Closing

Note: See TracTickets for help on using tickets.