[Scummvm-git-logs] scummvm master -> 18b8335fead97f4246ce264d6852b5e0442abac5

[tag2015]

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

Summary:
18b8335fea AGS: Engine: fixed the bw-compat SetObjectFrame() behavior


Commit: 18b8335fead97f4246ce264d6852b5e0442abac5
    https://github.com/scummvm/scummvm/commit/18b8335fead97f4246ce264d6852b5e0442abac5
Author: Walter Agazzi (walter.agazzi at protonmail.com)
Date: 2023-08-14T18:34:58+02:00

Commit Message:
AGS: Engine: fixed the bw-compat SetObjectFrame() behavior

This was broken by eb376e6

1. Treating negative loop and frame as an instruction to keep latest object's
loop and frame values was introduced around AGS 2.70 by Object.SetView(),
but this behavior also propagated to SetObjectFrame() command.

2. Removed clamping of loop and frame values inside Object.SetView(),
as that prevented applying a historical behavior of SetObjectFrame(),
where any invalid loop and frame value would be changed to just 0.

3. Don't quit the game in case of invalid loop and frame, but fallback
to default 0 value, and print a warning.

Also removed some redundant assignments to RoomObject,
possibly a result of a commit merge from another branch.

>From upstream 66bb18f0224eebfb2a130395e9b18ce59b584e13
This is the 3.6.1 branch, but the fix is required to solve a game-breaking
bug in Chronicle of Innsmouth

Changed paths:
    engines/ags/engine/ac/global_object.cpp
    engines/ags/engine/ac/object.cpp


diff --git a/engines/ags/engine/ac/global_object.cpp b/engines/ags/engine/ac/global_object.cpp
index 1582d639061..6480b661eb2 100644
--- a/engines/ags/engine/ac/global_object.cpp
+++ b/engines/ags/engine/ac/global_object.cpp
@@ -154,15 +154,30 @@ void SetObjectFrame(int obn, int viw, int lop, int fra) {
 	viw--;
 	if (viw < 0 || viw >= _GP(game).numviews) quitprintf("!SetObjectFrame: invalid view number used (%d, range is 0 - %d)", viw, _GP(game).numviews - 1);
 	if (_GP(views)[viw].numLoops == 0) quitprintf("!SetObjectFrame: view %d has no loops", viw);
-	if (lop < 0 || lop >= _GP(views)[viw].numLoops) quitprintf("!SetObjectFrame: invalid loop number used for view %d (%d, range is 0 - %d)", viw, lop, _GP(views)[viw].numLoops - 1);
-	// historically AGS let user to pass literally any positive invalid frame value by silently reassigning it to zero...
+
+	auto &obj = _G(objs)[obn];
+
+	// Previous version of Object.SetView had negative loop and frame mean "use latest values",
+	// which also caused SetObjectFrame to act similarly, starting with 2.70.
+	if ((_GP(game).options[OPT_BASESCRIPTAPI] < kScriptAPI_v360) && (_G(loaded_game_file_version) >= kGameVersion_270)) {
+		if (lop < 0)
+			lop = obj.loop;
+		if (fra < 0)
+			fra = obj.frame;
+	}
+
+	// Fixup invalid loop & frame numbers by using default 0 value
+	if (lop < 0 || lop >= _GP(views)[viw].numLoops) {
+		debug_script_warn("SetObjectFrame: invalid loop number used for view %d (%d, range is 0 - %d)", viw, lop, _GP(views)[viw].numLoops - 1);
+		lop = 0;
+	}
+
 	if (fra < 0 || fra >= _GP(views)[viw].loops[lop].numFrames) {
-		if (_GP(views)[viw].loops[lop].numFrames == 0)
-			debug_script_warn("SetObjectFrame: specified loop %d has no frames, will fallback to dummy frame", lop);
-		else
-			debug_script_warn("SetObjectFrame: frame index out of range (%d, must be 0 - %d), set to 0", fra, _GP(views)[viw].loops[lop].numFrames - 1);
+		debug_script_warn("SetObjectFrame: frame index out of range (%d, must be 0 - %d)", fra, _GP(views)[viw].loops[lop].numFrames - 1);
 		fra = 0; // NOTE: we have 1 dummy frame allocated for empty loops
 	}
+
+	// Current engine's object data limitation by uint16_t
 	if (viw > UINT16_MAX || lop > UINT16_MAX || fra > UINT16_MAX) {
 		debug_script_warn("Warning: object's (id %d) view/loop/frame (%d/%d/%d) is outside of internal range (%d/%d/%d), reset to no view",
 			obn, viw + 1, lop, fra, UINT16_MAX + 1, UINT16_MAX, UINT16_MAX);
@@ -170,21 +185,15 @@ void SetObjectFrame(int obn, int viw, int lop, int fra) {
 		return;
 	}
 
-	_G(objs)[obn].view = (uint16_t)viw;
-	if (lop >= 0)
-		_G(objs)[obn].loop = (uint16_t)lop;
-	if (fra >= 0)
-		_G(objs)[obn].frame = (uint16_t)fra;
-
-	_G(objs)[obn].view = viw;
-	_G(objs)[obn].loop = lop;
-	_G(objs)[obn].frame = fra;
-	_G(objs)[obn].cycling = 0;
+	obj.view = viw;
+	obj.loop = lop;
+	obj.frame = fra;
+	obj.cycling = 0; // reset anim
 	int pic = _GP(views)[viw].loops[lop].frames[fra].pic;
-	_G(objs)[obn].num = Math::InRangeOrDef<uint16_t>(pic, 0);
+	obj.num = Math::InRangeOrDef<uint16_t>(pic, 0);
 	if (pic > UINT16_MAX)
 		debug_script_warn("Warning: object's (id %d) sprite %d is outside of internal range (%d), reset to 0", obn, pic, UINT16_MAX);
-	CheckViewFrame(viw, _G(objs)[obn].loop, _G(objs)[obn].frame);
+	CheckViewFrame(viw, obj.loop, obj.frame);
 }
 
 // pass trans=0 for fully solid, trans=100 for fully transparent
diff --git a/engines/ags/engine/ac/object.cpp b/engines/ags/engine/ac/object.cpp
index c3d52943c5a..c91b4b8f4b3 100644
--- a/engines/ags/engine/ac/object.cpp
+++ b/engines/ags/engine/ac/object.cpp
@@ -88,16 +88,6 @@ void Object_RemoveTint(ScriptObject *objj) {
 }
 
 void Object_SetView(ScriptObject *objj, int view, int loop, int frame) {
-	if (_GP(game).options[OPT_BASESCRIPTAPI] < kScriptAPI_v360) { // Previous version of SetView had negative loop and frame mean "use latest values"
-		auto &obj = _G(objs)[objj->id];
-		if (loop < 0) loop = obj.loop;
-		if (frame < 0) frame = obj.frame;
-		const int vidx = view - 1;
-		if (vidx < 0 || vidx >= _GP(game).numviews) quit("!Object_SetView: invalid view number used");
-		loop = Math::Clamp(loop, 0, (int)_GP(views)[vidx].numLoops - 1);
-		frame = Math::Clamp(frame, 0, (int)_GP(views)[vidx].loops[loop].numFrames - 1);
-	}
-
 	SetObjectFrame(objj->id, view, loop, frame);
 }