[Scummvm-git-logs] scummvm master -> 86cc6fe6274d81bbdbc9d94cbde50848722cdf1d

[sev-]

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

Summary:
86cc6fe627 DIRECTOR: Fix saving members created at runtime


Commit: 86cc6fe6274d81bbdbc9d94cbde50848722cdf1d
    https://github.com/scummvm/scummvm/commit/86cc6fe6274d81bbdbc9d94cbde50848722cdf1d
Author: Gianluca Boiano (morf3089 at gmail.com)
Date: 2026-07-23T16:16:31+02:00

Commit Message:
DIRECTOR: Fix saving members created at runtime

writeCast() skipped empty CAS* slots, collapsing the positional array:
a member duplicated to id 30 reloaded as id 6. Runtime-created members
also shared the child resources that are re-serialized from member
state (STXT, BITD, CLUT, SCVW), so editing a duplicate could overwrite
its source on save; clone those entries instead. Duplicated palettes
also copied struct memory as color data and shared the color array
with their source, double-freeing it on destruction.

Fixes duplicated members changing id and text on reload (verified with
duplicate/modify/save/reload on text_d5_win.dir; bitmap and palette
duplicates verified structurally)

Changed paths:
    engines/director/archive-save.cpp
    engines/director/cast.cpp
    engines/director/castmember/palette.cpp


diff --git a/engines/director/archive-save.cpp b/engines/director/archive-save.cpp
index 07ec1a0ccf3..c77f9c974fc 100644
--- a/engines/director/archive-save.cpp
+++ b/engines/director/archive-save.cpp
@@ -334,10 +334,10 @@ bool RIFXArchive::writeCast(Common::SeekableWriteStream *writeStream, uint32 off
 	debugCN(5, kDebugSaving, "'CASt' indexes: [");
 	for (uint32 i = 0; i <= maxCastId; i++) {
 		uint32 castIndex = castIndexes.getValOrDefault(i, 0);
-		if (castIndex) {
+		// CAS* is a positional array: empty slots are written as 0
+		writeStream->writeUint32BE(castIndex);
+		if (castIndex)
 			debugCN(5, kDebugSaving, (i == 0 ? "%d" : ", %d"), castIndex);
-			writeStream->writeUint32BE(castIndex);
-		}
 	}
 	debugC(5, kDebugSaving, "]");
 
@@ -381,21 +381,38 @@ Common::Array<Resource *> RIFXArchive::rebuildResources(Movie *movie) {
 				if (!res) {
 					// If the castId is new, create a new resource
 					// Assigning the next available index to the resource
-					res = &castResMap[_resources.size()];
+					uint16 newResIndex = _resources.size();
+					res = &castResMap[newResIndex];
 					res->tag = MKTAG('C', 'A', 'S', 't');
 					res->accessed = true;
 
 					res->libResourceId = cast->_libResourceId;
-					res->children = jt._value->_children;
-					res->index = _resources.size();
+					res->index = newResIndex;
 					res->castId = jt._value->getID() - cast->_castArrayStart;
+					_resources.push_back(res);
 
+					// Clone the child resources that are re-serialized from
+					// their owning member's state: the duplicate must not
+					// share them with its source
+					res->children.clear();
 					for (auto child : jt._value->_children) {
+						if (child.tag == MKTAG('S', 'T', 'X', 'T') || child.tag == MKTAG('B', 'I', 'T', 'D') ||
+								child.tag == MKTAG('C', 'L', 'U', 'T') || child.tag == MKTAG('S', 'C', 'V', 'W')) {
+							uint16 childIndex = _resources.size();
+							Resource &newChild = _types[child.tag][childIndex];
+							newChild = child;
+							newChild.index = childIndex;
+							_resources.push_back(&newChild);
+							child.index = childIndex;
+						}
+						res->children.push_back(child);
+					}
+
+					for (auto child : res->children) {
 						_keyData[child.tag][res->index].push_back(child.index);
 						_keyTableUsedCount += 1;
 						_keyTableEntryCount += 1;
 					}
-					_resources.push_back(res);
 
 					debugC(5, kDebugSaving, "RIFXArchive::rebuildResources(): new 'CASt' resource added");
 				} else {
diff --git a/engines/director/cast.cpp b/engines/director/cast.cpp
index 597cf308c7b..666c348369d 100644
--- a/engines/director/cast.cpp
+++ b/engines/director/cast.cpp
@@ -274,6 +274,10 @@ bool Cast::duplicateCastMember(CastMember *source, CastMemberInfo *info, int tar
 	if (!source)
 		return true;
 	CastMember *target = source->duplicate(this, targetId);
+	// Some duplicate() implementations don't carry the child resource
+	// references; they only make sense within the same archive
+	if (target->_children.empty() && source->getCast() == this)
+		target->_children = source->_children;
 
 	if (info) {
 		CastMemberInfo *newInfo = new CastMemberInfo(*info);
diff --git a/engines/director/castmember/palette.cpp b/engines/director/castmember/palette.cpp
index 7534a8cb4ef..60d12236af1 100644
--- a/engines/director/castmember/palette.cpp
+++ b/engines/director/castmember/palette.cpp
@@ -42,7 +42,13 @@ PaletteCastMember::PaletteCastMember(Cast *cast, uint16 castId, PaletteCastMembe
 	source.load();
 	_loaded = true;
 
-	_palette = source._palette ? new PaletteV4(*source._palette) : nullptr;
+	if (source._palette) {
+		byte *colors = new byte[source._palette->length * 3];
+		memcpy(colors, source._palette->palette, source._palette->length * 3);
+		_palette = new PaletteV4(source._palette->id, colors, source._palette->length);
+	} else {
+		_palette = nullptr;
+	}
 }
 
 PaletteCastMember::PaletteCastMember(Cast *cast, uint16 castId, byte *paletteData, PaletteV4 *pal)
@@ -52,12 +58,8 @@ PaletteCastMember::PaletteCastMember(Cast *cast, uint16 castId, byte *paletteDat
 	_loaded = true;
 }
 
-// Need to make a deep copy
 CastMember *PaletteCastMember::duplicate(Cast *cast, uint16 castId) {
-	byte *buf = (byte *)malloc(_palette->length);
-	memcpy(buf, _palette, _palette->length);
-
-	return (CastMember *)(new PaletteCastMember(cast, castId, buf, _palette));
+	return new PaletteCastMember(cast, castId, *this);
 }
 
 PaletteCastMember::~PaletteCastMember() {