[Scummvm-git-logs] scummvm master -> 1116b30fa03c63c34ce242fd190119a9e7add33f

mduggan mgithub at guarana.org
Fri May 22 23:19:59 UTC 2020


This automated email contains information about 7 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
8f5c5c8a9a ULTIMA8: Remove memory leak from debug object
86e5417d90 ULTIMA8: Fix leak when overriding builtin data
71c453aa5b ULTIMA8: Remove pointless override
1e8344919d ULTIMA8: Drop missing music track from error to warning
83fa88632a ULTIMA8: Try to fix use-after-free in pathfinding
5b64b326fa ULTIMA8: Fix stacking of dead man's elbow
1116b30fa0 ULTIMA8: Small refactor to remove duplicate code


Commit: 8f5c5c8a9a55cf0375a68538a5259e4a842dbfdc
    https://github.com/scummvm/scummvm/commit/8f5c5c8a9a55cf0375a68538a5259e4a842dbfdc
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Remove memory leak from debug object

Changed paths:
    engines/ultima/ultima8/usecode/uc_machine.cpp


diff --git a/engines/ultima/ultima8/usecode/uc_machine.cpp b/engines/ultima/ultima8/usecode/uc_machine.cpp
index 116f9890e6..516d29c67d 100644
--- a/engines/ultima/ultima8/usecode/uc_machine.cpp
+++ b/engines/ultima/ultima8/usecode/uc_machine.cpp
@@ -372,6 +372,7 @@ void UCMachine::execProcess(UCProcess *p) {
 					ARG_UC_PTR(iptr);
 					uint16 testItemId = ptrToObject(iptr);
 					testItem = getItem(testItemId);
+					delete [] args;
 				}
 				perr << "Unhandled intrinsic << " << func << " \'" << _convUse->intrinsics()[func] << "\'? (";
 				if (testItem) {


Commit: 86e5417d9067dbf033cf005e35a86bb74b0e9d03
    https://github.com/scummvm/scummvm/commit/86e5417d9067dbf033cf005e35a86bb74b0e9d03
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Fix leak when overriding builtin data

Changed paths:
    engines/ultima/ultima8/filesys/file_system.cpp


diff --git a/engines/ultima/ultima8/filesys/file_system.cpp b/engines/ultima/ultima8/filesys/file_system.cpp
index dc17daa848..b2c143c84c 100644
--- a/engines/ultima/ultima8/filesys/file_system.cpp
+++ b/engines/ultima/ultima8/filesys/file_system.cpp
@@ -58,6 +58,9 @@ IDataSource *FileSystem::ReadFile(const string &vfn, bool is_text) {
 	if (!_allowDataOverride && data)
 		return data;
 
+	if (data)
+		delete data;
+
 	Common::SeekableReadStream *readStream;
 	if (!rawOpen(readStream, vfn))
 		return nullptr;


Commit: 71c453aa5bdcd6d66cd8095b47e973093471c96c
    https://github.com/scummvm/scummvm/commit/71c453aa5bdcd6d66cd8095b47e973093471c96c
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Remove pointless override

Changed paths:
    engines/ultima/ultima8/audio/music_flex.h


diff --git a/engines/ultima/ultima8/audio/music_flex.h b/engines/ultima/ultima8/audio/music_flex.h
index 870b83be8b..90041edae6 100644
--- a/engines/ultima/ultima8/audio/music_flex.h
+++ b/engines/ultima/ultima8/audio/music_flex.h
@@ -66,10 +66,6 @@ public:
 	void uncache(uint32 index) override;
 	bool isCached(uint32 index) const override;
 
-	uint8 *getRawObject(uint32 index, uint32 *sizep = 0) {
-		return Archive::getRawObject(index, sizep);
-	}
-
 private:
 	SongInfo   *_info[128];
 	XMidiData  **_songs;


Commit: 1e8344919d60936dff5c3f13da42fb47a1f09a50
    https://github.com/scummvm/scummvm/commit/1e8344919d60936dff5c3f13da42fb47a1f09a50
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Drop missing music track from error to warning

Fixes crash when the First Acolyte is killed.

Changed paths:
    engines/ultima/ultima8/audio/music_flex.cpp


diff --git a/engines/ultima/ultima8/audio/music_flex.cpp b/engines/ultima/ultima8/audio/music_flex.cpp
index 5b8b924f37..b70e5e9347 100644
--- a/engines/ultima/ultima8/audio/music_flex.cpp
+++ b/engines/ultima/ultima8/audio/music_flex.cpp
@@ -76,7 +76,10 @@ void MusicFlex::cache(uint32 index) {
 	uint32 size;
 	uint8 *data = getRawObject(index, &size);
 	if (!data) {
-		error("Unable to cache song %d from sound/music.flx", index);
+		// Note: multiple sorcerer scenes (such as MALCHIR::03F2)
+		// request track 122, which is blank in the Gold Edition
+		// music flex.
+		warning("Unable to cache song %d from sound/music.flx", index);
 		return;
 	}
 	_songs[index] = new XMidiData(data, size);


Commit: 83fa88632a04b020b3e078924a3a1f377602d521
    https://github.com/scummvm/scummvm/commit/83fa88632a04b020b3e078924a3a1f377602d521
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Try to fix use-after-free in pathfinding

This code is a bit hard to track through though so I'm not 100% sure I got it
right.  Have done a lot of playtesting with the code like this and it seems ok?

Changed paths:
    engines/ultima/ultima8/world/actors/pathfinder.cpp
    engines/ultima/ultima8/world/actors/pathfinder.h


diff --git a/engines/ultima/ultima8/world/actors/pathfinder.cpp b/engines/ultima/ultima8/world/actors/pathfinder.cpp
index 71c8cc5ca5..1fbfa51fb5 100644
--- a/engines/ultima/ultima8/world/actors/pathfinder.cpp
+++ b/engines/ultima/ultima8/world/actors/pathfinder.cpp
@@ -122,15 +122,15 @@ Pathfinder::Pathfinder() : _actor(nullptr), _targetItem(nullptr),
 
 Pathfinder::~Pathfinder() {
 #if 1
-	pout << "~Pathfinder: " << _nodeList.size() << " _nodes, "
-	     << expandednodes << " expanded _nodes in " << _expandTime << "ms." << Std::endl;
+	pout << "~Pathfinder: " << _cleanupNodes.size() << " nodes to clean up, "
+	     << expandednodes << " expanded nodes in " << _expandTime << "ms." << Std::endl;
 #endif
 
 	// clean up _nodes
-	Std::list<PathNode *>::iterator iter;
-	for (iter = _nodeList.begin(); iter != _nodeList.end(); ++iter)
+	Std::vector<PathNode *>::iterator iter;
+	for (iter = _cleanupNodes.begin(); iter != _cleanupNodes.end(); ++iter)
 		delete *iter;
-	_nodeList.clear();
+	_cleanupNodes.clear();
 }
 
 void Pathfinder::init(Actor *actor_, PathfindingState *state) {
@@ -187,7 +187,7 @@ bool Pathfinder::alreadyVisited(int32 x, int32 y, int32 z) const {
 	return false;
 }
 
-bool Pathfinder::checkTarget(PathNode *node) const {
+bool Pathfinder::checkTarget(const PathNode *node) const {
 	// TODO: these ranges are probably a bit too high,
 	// but otherwise it won't work properly yet -wjp
 	if (_targetItem) {
@@ -358,7 +358,6 @@ static void drawpath(PathNode *to, uint32 rgb, bool done) {
 void Pathfinder::newNode(PathNode *oldnode, PathfindingState &state,
                          unsigned int steps) {
 	PathNode *newnode = new PathNode();
-	_nodeList.push_back(newnode); // for garbage collection
 	newnode->state = state;
 	newnode->parent = oldnode;
 	newnode->depth = oldnode->depth + 1;
@@ -536,7 +535,6 @@ bool Pathfinder::pathfind(Std::vector<PathfindingAction> &path) {
 	startnode->parent = nullptr;
 	startnode->depth = 0;
 	startnode->stepsfromparent = 0;
-	_nodeList.push_back(startnode);
 	_nodes.push(startnode);
 
 	unsigned int expandedNodes = 0;
@@ -546,7 +544,9 @@ bool Pathfinder::pathfind(Std::vector<PathfindingAction> &path) {
 	uint32 starttime = g_system->getMillis();
 
 	while (expandedNodes < NODELIMIT_MAX && !_nodes.empty() && !found) {
-		PathNode *node = _nodes.top();
+		// Take a copy here as the pop() below deletes the old node
+		PathNode *node = new PathNode(*_nodes.top());
+		_cleanupNodes.push_back(node);
 		_nodes.pop();
 
 #if 0
@@ -559,7 +559,7 @@ bool Pathfinder::pathfind(Std::vector<PathfindingAction> &path) {
 			// done!
 
 			// find path length
-			PathNode *n = node;
+			const PathNode *n = node;
 			unsigned int length = 0;
 			while (n->parent) {
 				n = n->parent;
diff --git a/engines/ultima/ultima8/world/actors/pathfinder.h b/engines/ultima/ultima8/world/actors/pathfinder.h
index f0a74ff677..59a3df6e5b 100644
--- a/engines/ultima/ultima8/world/actors/pathfinder.h
+++ b/engines/ultima/ultima8/world/actors/pathfinder.h
@@ -96,14 +96,15 @@ protected:
 	Std::list<PathfindingState> _visited;
 	Std::priority_queue<PathNode *, Std::vector<PathNode *>, PathNodeCmp> _nodes;
 
-	Std::list<PathNode *> _nodeList;
+	/** List of nodes for garbage collection later and order is not important */
+	Std::vector<PathNode *> _cleanupNodes;
 
 	bool alreadyVisited(int32 x, int32 y, int32 z) const;
 	void newNode(PathNode *oldnode, PathfindingState &state,
 				 unsigned int steps);
 	void expandNode(PathNode *node);
 	unsigned int costHeuristic(PathNode *node);
-	bool checkTarget(PathNode *node) const;
+	bool checkTarget(const PathNode *node) const;
 };
 
 } // End of namespace Ultima8


Commit: 5b64b326fa9b1b8f13c4961468cb4d656bcb0c68
    https://github.com/scummvm/scummvm/commit/5b64b326fa9b1b8f13c4961468cb4d656bcb0c68
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Fix stacking of dead man's elbow

Changed paths:
    engines/ultima/ultima8/world/item.cpp


diff --git a/engines/ultima/ultima8/world/item.cpp b/engines/ultima/ultima8/world/item.cpp
index a988ffc506..8400a18466 100644
--- a/engines/ultima/ultima8/world/item.cpp
+++ b/engines/ultima/ultima8/world/item.cpp
@@ -1736,6 +1736,14 @@ bool Item::canReach(Item *other, int range,
 	                                  getObjId(), other->getObjId());
 }
 
+
+/**
+ * A helper function to check if both frames are in the
+ * same range (inclusive) for the merge check below */
+static inline bool bothInRange(uint32 x, uint32 y, uint32 lo, uint32 hi) {
+	return (x >= lo && x <= hi && y >= lo && y <= hi);
+}
+
 bool Item::canMergeWith(Item *other) {
 	// can't merge with self
 	if (other->getObjId() == getObjId()) return false;
@@ -1751,24 +1759,52 @@ bool Item::canMergeWith(Item *other) {
 	uint32 frame2 = other->getFrame();
 	if (frame1 == frame2) return true;
 
-	// special cases: necromancy reagents, _shape 395
-	// blood: _frame 0-5
-	// bone: _frame 6-7
-	// wood: _frame 8
-	// dirt: _frame 9
-	// ex.hood: _frame 10-12
-	// blackmoor: _frame 14-15
-	if (CoreApp::get_instance()->getGameInfo()->_type == GameInfo::GAME_U8) {
-		if (getShape() != 395) return false;
-
-		if (frame1 <= 5 && frame2 <= 5)
-			return true;
-		if (frame1 >= 6 && frame1 <= 7 && frame2 >= 6 && frame2 <= 7)
-			return true;
-		if (frame1 >= 10 && frame1 <= 12 && frame2 >= 10 && frame2 <= 12)
-			return true;
-		if (frame1 >= 14 && frame1 <= 15 && frame2 >= 14 && frame2 <= 15)
-			return true;
+	// special cases:
+	// necromancy reagents (shape 395)
+	// 		blood: frame 0-5
+	// 		bone: frame 6-7
+	// 		wood: frame 8
+	// 		dirt: frame 9
+	// 		ex.hood: frame 10-12
+	// 		blackmoor: frame 14-15
+	// 		dead man's elbow: frame 16-20
+	// sorcery reagents (shape 398).
+	// Disabled because the usecode doesn't support saying how many there are.
+	//		volcanic ash: frame 0-1
+	//		pumice: frame 2-5
+	//		obsidian: 6-9
+	//		iron: 10-13
+	//		brimstone: 14-17
+	// 		daemon bones: 18-20
+	// 3. ether reagents (shape 399) (also not supported in usecode)
+	//
+	if (GAME_IS_U8) {
+		if (getShape() == 395) {
+			if (bothInRange(frame1, frame2, 0, 5))
+				return true;
+			if (bothInRange(frame1, frame2, 6, 7))
+				return true;
+			if (bothInRange(frame1, frame2, 10, 12))
+				return true;
+			if (bothInRange(frame1, frame2, 14, 15))
+				return true;
+			if (bothInRange(frame1, frame2, 16, 20))
+				return true;
+		}
+		/*if (getShape() == 398) {
+			if (bothInRange(frame1, frame2, 0, 1))
+				return true;
+			if (bothInRange(frame1, frame2, 2, 5))
+				return true;
+			if (bothInRange(frame1, frame2, 6, 9))
+				return true;
+			if (bothInRange(frame1, frame2, 10, 13))
+				return true;
+			if (bothInRange(frame1, frame2, 14, 17))
+				return true;
+			if (bothInRange(frame1, frame2, 18, 20))
+				return true;
+		}*/
 	}
 	return false;
 }


Commit: 1116b30fa03c63c34ce242fd190119a9e7add33f
    https://github.com/scummvm/scummvm/commit/1116b30fa03c63c34ce242fd190119a9e7add33f
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2020-05-23T08:19:18+09:00

Commit Message:
ULTIMA8: Small refactor to remove duplicate code

Changed paths:
    engines/ultima/ultima8/gumps/ask_gump.cpp
    engines/ultima/ultima8/gumps/bark_gump.cpp
    engines/ultima/ultima8/gumps/bark_gump.h


diff --git a/engines/ultima/ultima8/gumps/ask_gump.cpp b/engines/ultima/ultima8/gumps/ask_gump.cpp
index 778eb41ce1..63eb2786db 100644
--- a/engines/ultima/ultima8/gumps/ask_gump.cpp
+++ b/engines/ultima/ultima8/gumps/ask_gump.cpp
@@ -23,6 +23,7 @@
 
 #include "ultima/ultima8/misc/pent_include.h"
 #include "ultima/ultima8/gumps/ask_gump.h"
+#include "ultima/ultima8/gumps/bark_gump.h"
 #include "ultima/ultima8/gumps/widgets/button_widget.h"
 #include "ultima/ultima8/usecode/uc_list.h"
 #include "ultima/ultima8/usecode/uc_machine.h"
@@ -50,22 +51,7 @@ AskGump::~AskGump() {
 // Init the gump, call after construction
 void AskGump::InitGump(Gump *newparent, bool take_focus) {
 	// OK, this is a bit of a hack, but it's how it has to be
-	int fontnum;
-	if (_owner == 1) fontnum = 6;
-	else if (_owner > 256) fontnum = 8;
-	else switch (_owner % 3) {
-		case 1:
-			fontnum = 5;
-			break;
-
-		case 2:
-			fontnum = 7;
-			break;
-
-		default:
-			fontnum = 0;
-			break;
-		}
+	int fontnum = BarkGump::dialogFontForActor(_owner);
 
 	int px = 0, py = 0;
 
diff --git a/engines/ultima/ultima8/gumps/bark_gump.cpp b/engines/ultima/ultima8/gumps/bark_gump.cpp
index 8d51365e6c..7449720918 100644
--- a/engines/ultima/ultima8/gumps/bark_gump.cpp
+++ b/engines/ultima/ultima8/gumps/bark_gump.cpp
@@ -50,24 +50,24 @@ BarkGump::BarkGump(uint16 owner, const Std::string &msg, uint32 speechShapeNum)
 BarkGump::~BarkGump(void) {
 }
 
-void BarkGump::InitGump(Gump *newparent, bool take_focus) {
+int BarkGump::dialogFontForActor(uint16 actor) {
 	// OK, this is a bit of a hack, but it's how it has to be
-	int fontnum;
-	if (_owner == 1) fontnum = 6;
-	else if (_owner > 256) fontnum = 8;
-	else switch (_owner % 3) {
-		case 1:
-			fontnum = 5;
-			break;
-
-		case 2:
-			fontnum = 7;
-			break;
-
-		default:
-			fontnum = 0;
-			break;
-		}
+	if (actor == 1)
+		return 6;
+	if (actor > 256)
+		return 8;
+	switch (actor % 3) {
+	case 1:
+		return 5;
+	case 2:
+		return 7;
+	default:
+		return 0;
+	}
+}
+
+void BarkGump::InitGump(Gump *newparent, bool take_focus) {
+	int fontnum = dialogFontForActor(_owner);
 
 	// This is a hack. We init the gump twice...
 	ItemRelativeGump::InitGump(newparent, take_focus);
diff --git a/engines/ultima/ultima8/gumps/bark_gump.h b/engines/ultima/ultima8/gumps/bark_gump.h
index 5a04083388..9d67bcbe8a 100644
--- a/engines/ultima/ultima8/gumps/bark_gump.h
+++ b/engines/ultima/ultima8/gumps/bark_gump.h
@@ -57,6 +57,9 @@ public:
 	// Init the gump, call after construction
 	void        InitGump(Gump *newparent, bool take_focus = true) override;
 
+	/// Get the font that should be used from dialog from this actor
+	static int dialogFontForActor(uint16 actor);
+
 protected:
 	//! show next text.
 	//! returns false if no more text available




More information about the Scummvm-git-logs mailing list