[Scummvm-cvs-logs] SF.net SVN: scummvm:[46068] scummvm/trunk/engines/draci

[spalek at users.sourceforge.net]

Revision: 46068
          http://scummvm.svn.sourceforge.net/scummvm/?rev=46068&view=rev
Author:   spalek
Date:     2009-11-22 09:13:05 +0000 (Sun, 22 Nov 2009)

Log Message:
-----------
Fix SIGSEGV by an absolutely brutally horrible hack

I have thoroughly documented why this hack is needed and added ideas how to
fix it properly.

Modified Paths:
--------------
    scummvm/trunk/engines/draci/game.cpp
    scummvm/trunk/engines/draci/walking.h

Modified: scummvm/trunk/engines/draci/game.cpp
===================================================================
--- scummvm/trunk/engines/draci/game.cpp	2009-11-22 09:01:57 UTC (rev 46067)
+++ scummvm/trunk/engines/draci/game.cpp	2009-11-22 09:13:05 UTC (rev 46068)
@@ -1155,12 +1155,73 @@
 }
 
 void Game::deleteObjectAnimations() {
-	for (uint i = 0; i < _info._numObjects; ++i) {
+	for (uint i = 1; i < _info._numObjects; ++i) {
 		GameObject *obj = &_objects[i];
-		if (i != 0 && (obj->_location == getPreviousRoomNum())) {
+		if (obj->_location == getPreviousRoomNum()) {
 			obj->deleteAnims();
 		}
 	}
+	
+	// WORKAROUND
+	//
+	// An absolutely horrible hack follows.  The current memory management
+	// is completely broken and it needs to be seriously hacked to work at
+	// all.  The problem is caching sound samples in BArchive and clearing
+	// all caches when entering a new location.  The animation sequences
+	// store pointers to samples owned by the BArchive cache, which gets
+	// invalidated during the location change.  If an animation sequence
+	// survives location change and refers to any sound sample, we get
+	// SIGSEGV when next played, because this sound sample will have been
+	// deallocated by the cache although still referred to by the animation.
+	//
+	// Caveat: when I tried to perform a deep copy and make the animation
+	// object own the sound samples and deallocate them when the animation
+	// has been deleted, I get an almost immediate SIGSEGV in runWrapper()
+	// on many objects, because often an animation graphically ends and is
+	// stopped, but the last sound sample still plays for a short while
+	// afterwards.  If the sound sample is cached, it's OK, but if it's
+	// deallocated with releaseAnims==true, we get SIGSEGV in the sound
+	// mixer.  This problem doesn't occur when changing locations, because
+	// there we first explicitly stop all playing sounds.
+	//
+	// The loop above deallocates all animations corresponding to non-hero
+	// objects.  Hero is special, because the first ~20 animations are
+	// standard and loaded for the whole game (standing, walking, etc.).
+	// They are loaded by the GPL2 init routine for object hero.  Luckily
+	// the animations don't refer to any sound samples.  The remaining
+	// animations thus must be deallocated manually, otherwise they won't
+	// be re-loaded next time assuming that they are already correctly
+	// loaded.
+	//
+	// mini-TODO: integrate this deletion into deleteAnim() an optional
+	// parameter with starting index for deletion.
+	//
+	// Why this only occurs for sound samples and not sprites?  This bug is
+	// concealed by a complete coincidence, that all sprites are stored
+	// column-wise and our class Sprite detects this and creates a local
+	// copy.  If this wasn't the case, each animation (not just with sound
+	// samples) would fail and preserving the ~20 hero animations wouldn't
+	// work either.
+	//
+	// TODO: maybe simply deallocate everything except for those ~20 hero
+	// animations instead of listing what to deallocate.  maybe simply
+	// deallocate everything; reloading isn't that expensive.
+	//
+	// TODO: completely rewrite the resource management.  maybe implement
+	// usage counters?  maybe completely ignore the GPL2 hints and manage
+	// memory completely on my own?
+	//
+	// URGENT TODO: if a game item is in the hero's hands when changing
+	// locations, its animations survive and we get assert in
+	// AnimationManager::load().
+	GameObject *dragon = &_objects[kDragonObject];
+	for (uint i = dragon->_anim.size() - 1; i >= kFirstTemporaryAnimation; --i) {
+		dragon->_anim.back()->del();
+		dragon->_anim.pop_back();
+	}
+	if (dragon->_playingAnim >= kFirstTemporaryAnimation) {
+		dragon->_playingAnim = 0;
+	}
 }
 
 bool Game::enterNewRoom() {

Modified: scummvm/trunk/engines/draci/walking.h
===================================================================
--- scummvm/trunk/engines/draci/walking.h	2009-11-22 09:01:57 UTC (rev 46067)
+++ scummvm/trunk/engines/draci/walking.h	2009-11-22 09:13:05 UTC (rev 46068)
@@ -96,7 +96,9 @@
 	kMoveLeftRight, kMoveRightLeft, kMoveUpStopLeft, kMoveUpStopRight,
 	kLastTurning = kMoveUpStopRight,
 
-	kSpeakRight, kSpeakLeft, kStopRight, kStopLeft
+	kSpeakRight, kSpeakLeft, kStopRight, kStopLeft,
+
+	kFirstTemporaryAnimation
 };
 
 class DraciEngine;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.