[Scummvm-git-logs] scummvm master -> 3b3b9538c59a18ddff847bfbf5ab17c86cd263dd

[mduggan]

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

Summary:
59df855849 ULTIMA8: Slight cleanup in World::clear
3b3b9538c5 ULTIMA8: Make popToContainer/popToEnd better match originals


Commit: 59df855849316c41828fb36e45de2f28e0c193e5
    https://github.com/scummvm/scummvm/commit/59df855849316c41828fb36e45de2f28e0c193e5
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2021-12-28T20:52:35+09:00

Commit Message:
ULTIMA8: Slight cleanup in World::clear

Use the existing list clear function for the ethereal list.

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


diff --git a/engines/ultima/ultima8/world/world.cpp b/engines/ultima/ultima8/world/world.cpp
index 83b7462d2ca..d1d90c0f220 100644
--- a/engines/ultima/ultima8/world/world.cpp
+++ b/engines/ultima/ultima8/world/world.cpp
@@ -72,8 +72,7 @@ void World::clear() {
 	}
 	_maps.clear();
 
-	while (!_ethereal.empty())
-		_ethereal.pop_front();
+	_ethereal.clear();
 
 	if (_currentMap)
 		delete _currentMap;


Commit: 3b3b9538c59a18ddff847bfbf5ab17c86cd263dd
    https://github.com/scummvm/scummvm/commit/3b3b9538c59a18ddff847bfbf5ab17c86cd263dd
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2021-12-28T20:52:35+09:00

Commit Message:
ULTIMA8: Make popToContainer/popToEnd better match originals

A followup to bug #13176.

One of the problems in the savegame on the bug is trying to pop items to
containers that don't exist.  This maybe never happens in the original, but if
it does the item will be popped off the ethereal list and discarded anyway,
which we were previously not doing.

This mistake meant there were objects left over on the ethereal list which
shouldn't be there.  They could probably get mixed up with other items by game
scripts, which is possibly the cause of the original bug.

In investigating, I also discovered that if a non-container is passed to the
original games the object will still be popped to the location of the
"container".  I'm not sure if this behavior is ever needed, but implemented in
case.

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 db14f16f685..e9f14e56adf 100644
--- a/engines/ultima/ultima8/world/item.cpp
+++ b/engines/ultima/ultima8/world/item.cpp
@@ -3459,61 +3459,90 @@ uint32 Item::I_popToCoords(const uint8 *args, unsigned int /*argsize*/) {
 
 uint32 Item::I_popToContainer(const uint8 *args, unsigned int /*argsize*/) {
 	ARG_NULL32(); // ARG_ITEM_FROM_PTR(item); // unused
-	ARG_CONTAINER_FROM_ID(container);
-
-	if (!container) {
-		perr << "Trying to pop item to invalid container (" << id_container << ")." << Std::endl;
-		return 0;
-	}
+	ARG_ITEM_FROM_ID(citem);
 
 	World *w = World::get_instance();
 
 	if (w->etherealEmpty())
 		return 0; // no items left on stack
 
-	uint16 _objId = w->etherealPeek();
-	Item *item = getItem(_objId);
+	uint16 objId = w->etherealPeek();
+	Item *item = getItem(objId);
 	if (!item) {
-		w->etherealRemove(_objId);
+		w->etherealRemove(objId);
 		return 0; // top item was invalid
 	}
 
-	item->moveToContainer(container);
+	Container *container = dynamic_cast<Container *>(citem);
+	if (container) {
+		item->moveToContainer(container);
+	} else if (citem) {
+		Point3 pt;
+		citem->getLocation(pt);
+		item->move(pt);
+	} else {
+		perr << "Trying to popToContainer to invalid container (" << id_citem << ")" << Std::endl;
+		item->dumpInfo();
+		// This object now has no home, destroy it - unless it doesn't think it's
+		// ethereal, in that case it is somehow there by mistake?
+		if (item->getFlags() & FLG_ETHEREAL) {
+			perr << "Destroying orphaned ethereal object (" << objId << ")" << Std::endl;
+			item->destroy();
+		} else {
+			perr << "Leaving orphaned ethereal object (" << objId << ")" << Std::endl;
+			w->etherealRemove(objId);
+		}
+	}
 
 	//! Anything else?
 
-	return _objId;
+	return objId;
 }
 
 uint32 Item::I_popToEnd(const uint8 *args, unsigned int /*argsize*/) {
 	ARG_NULL32(); // ARG_ITEM_FROM_PTR(item); // unused
-	ARG_CONTAINER_FROM_ID(container);
-
-	if (!container) {
-		perr << "Trying to pop item to invalid container (" << id_container << ")." << Std::endl;
-		return 0;
-	}
+	ARG_ITEM_FROM_ID(citem);
 
 	World *w = World::get_instance();
 
 	if (w->etherealEmpty())
 		return 0; // no items left on stack
 
-	uint16 _objId = w->etherealPeek();
-	Item *item = getItem(_objId);
+	uint16 objId = w->etherealPeek();
+	Item *item = getItem(objId);
 	if (!item) {
-		w->etherealRemove(_objId);
+		w->etherealRemove(objId);
 		return 0; // top item was invalid
 	}
 
-	item->moveToContainer(container);
+	// TODO: This should also pop to container if citem is type 7 in Crusader
+	Container *container = dynamic_cast<Container *>(citem);
+	if (container) {
+		item->moveToContainer(container);
+	} else if (citem) {
+		Point3 pt;
+		citem->getLocation(pt);
+		item->move(pt);
+	} else {
+		perr << "Trying to popToEnd to invalid container (" << id_citem << ")" << Std::endl;
+		item->dumpInfo();
+		// This object now has no home, destroy it - unless it doesn't think it's
+		// ethereal, in that case it is somehow there by mistake?
+		if (item->getFlags() & FLG_ETHEREAL) {
+			perr << "Destroying orphaned ethereal object (" << objId << ")" << Std::endl;
+			item->destroy();
+		} else {
+			perr << "Leaving orphaned ethereal object (" << objId << ")" << Std::endl;
+			w->etherealRemove(objId);
+		}
+	}
 
 	//! Anything else?
 
 	//! This should probably be different from I_popToContainer, but
 	//! how exactly?
 
-	return _objId;
+	return objId;
 }
 
 uint32 Item::I_move(const uint8 *args, unsigned int /*argsize*/) {