QFG4 floppy: Assertion Error celobj32.cpp:179 at the swamp

[Vhati]

ScummVM 2.1.0git3770-g15306581ab (Oct 18 2018 04:27:32)
Windows 7 64bit
QFG4 Floppy 1.1a + note patch (English)

After starting a new game, something about the wisp swamp screen is causing an assertion failure shortly after entering it. Arrival triggers an autosave, but the error ranges from immediately to a couple seconds later.

Restore the attached savegame and just stand still.

Popup
" " "
MinGW Runtime Assertion

Assertion failed!

Program: scummvm.exe
File: ../../src-master/src/engines/sci/graphics/celobj32.cpp, Line 179

Expression: _minX <= _maxX
" " "

Console
" " "
Running Quest for Glory IV: Shadows of Darkness (DOS/English)
ADLIB: Starting driver in SCI1 mode
kFileIO(fileExists) 18.scr -> 0
kFileIO(fileExists) 18.scr -> 0
kFileIO(fileExists) 18.scr -> 0
kFileIO(fileExists) flythru.seq -> 0
# Restore Menu
kValidPath() -> 0
# Walk west
kFileIO(fileExists) 18.scr -> 0
kGetCWD() -> /
Game name Glory save 2 desc Automatic Save ver
" " "

File - 5kb MD5 - Full MD5
RESOURCE.000 - f64fd6aa3977939a86ff30783dd677e1 - ff42260a665995a85aeb277ad80aac8a
RESOURCE.MAP - d10a4cc177d2091d744e2ad8c049b0ae - 3695b1b0a1d15f3d324ea9f0cc325245
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

[Vhati]

SavedGame - WalkWest

[Vhati]

Correction: Restore the attached savegame, walk west, then just stand still.

[digitall]

Assertion is here:
​https://github.com/scummvm/scummvm/blob/master/engines/sci/graphics/celobj32.cpp#L179

[Vhati]

Disabling autosave didn't help.
Adjusting speed to its lowest setting and running didn't help.

I was afraid I'd never get past this, but I seem to have some luck if, from the start, I never adjust the speed slider and run through. Still crashes if I backtrack to that area.

[Vhati]

Good news! This isn't quite fatal. Just very annoying.
The assertion failure popup has an "ignore" button: that works.

Players have to click "ignore" repeatedly, once for every invalid animation tick. Sometimes it's constant. Sometimes there's a lull that allows players a little time to click WALK, leap, etc. and mash "ignore" until the hero completes the action.

Having traversed the swamp eastward from the tomb side. I suspect the undead hands are at fault, given the timing and unevenness of the errors. And during the day, there are no wisps around to blame.

[bluegr]

This happens in the floppy version only. The CD version works fine.

[Vhati]

Investigated a bit.

• Walk to the swamp shore west of the goo.
• Find the plane with ~24 screen items.
  • vpl
• List the screen items on that plane.
  • vpi 0012:0003
• send {every 'hands' address} hide
  • "hands.0" ... "hands.5" address notation is convenient.
• send {one 'hands' address} show

All but one sank and rose without problems.

One hand near the back (0012:05a5, x:48 y:108 view:535 loop:5 cycler:cueEndLoop) was throwing assertion errors repeatedly the whole time it was submerged. It was fine while it was up. No where near the edge of the screen.

Hands are positioned with randomly chosen values from lists and given a randomly chosen loop. (script 10 - Swamp::init())

All the possible hand loops have a submerged cell 0 with dimensions: 1x1.


I brought the hand closer to the foreground ("send 0012:05a5 y 150"), and it stopped throwing errors.

I moved it far away ("send 0012:05a5 y 100"), and it threw errors again.


I can't set a good breakpoint for the error and don't know how to check its width.
I'm guessing the scaler is reducing the 1x1 dimensions to 0.

​Source: celobj32.cpp

_minX(targetRect.left),
_maxX(targetRect.right - 1),
[...]
assert(_minX <= _maxX);

I don't know why the CD edition *doesn't* throw errors. Diff didn't reveal any interesting changes. SCI Companion says the View is scalable in both editions.

I fired up the CD edition, hid all but one hand, and gave it the above x, y, and loop values. CD edition behaves.

[Vhati]

Oh. The Scaler class changed.


Floppy, script 64935 - Scaler::doit()

(method (doit &tmp temp0 temp1 temp2 temp3)
	(= temp2 (client scaleX?))
	(= temp3 (client scaleY?))
	(cond 
		((< (= temp0 (client y?)) backY) (= temp1 backSize))
		((> temp0 frontY) (= temp1 frontSize))
		(else (= temp1 (+ (/ (* slopeNum temp0) slopeDen) const)))
	)
	(if
		(or
			(!= temp2 (= temp1 (/ (* temp1 128) 100)))
			(!= temp3 temp1)
		)
		(client scaleX: temp1 scaleY: temp1)
	)
)

CD, script 64935 - Scaler::doit()

(properties
# added a property
	lastY -9999
)

# ...

(method (doit &tmp temp0 temp1 temp2 temp3)
	(if (!= (= temp0 (client y?)) lastY)
		(= lastY temp0)
		(= temp2 (client scaleX?))
		(= temp3 (client scaleY?))
		(cond 
			((< temp0 backY) (= temp1 backSize))
			((> temp0 frontY) (= temp1 frontSize))
			(else (= temp1 (+ (/ (* slopeNum temp0) slopeDen) const)))
		)
		(if
			(or
				(!= temp2 (= temp1 (/ (* temp1 128) 100)))
				(!= temp3 temp1)
			)
			(client scaleX: temp1 scaleY: temp1)
		)
	)
)

[Vhati]

The only functional difference I'm seeing between Scaler versions is CD avoids recalculating if the y position hasn't changed since last time. And I don't think hands ever move.


script 10 - Swamp::init()

(= local7 (IntArray with: 54 48 95 158 132))
(= local8 (IntArray with: 175 108 119 142 178))
# ...
(switch g11_myCurrentRoomNum
# ...
	(535
		(= temp0 0)
		(while (< temp0 5)
			((hands new:)
				init: temp0 (local7 at: temp0) (local8 at: temp0)
			)
			(++ temp0)
		)
	)
# ...
)

script 10 - hands::init()

	(method (init param1 param2 param3)
		(super init:)
		(= x param2)
		(= y param3)
		(= loop (Random 0 5))
		((Timer new:) setReal: self (Random 5 25))
		(self
			signal: (| (self signal?) $0001)
			setScaler: Scaler 100 60 125 65
		)
	)

hands is a TargActor, whose ancestry is Feature/View/Prop/Actor.
Prop::setScaler() basically forwards args to Scaler::init(self, &rest).


script 64935 - Scaler::init()

(method (init param1 param2 param3 param4 param5)
	(if argc
		(= client param1)
		(= frontSize param2)
		(= backSize param3)
		(= frontY param4)
		(= backY param5)
	)
	(= slopeNum (- frontSize backSize))
	(if (not (= slopeDen (- frontY backY)))
		(proc64921_0 {<Scaler> frontY cannot be equal to backY})
		(return 0)
	)
	(= const (- backSize (/ (* slopeNum backY) slopeDen)))
	(return (self doit:))
)

A hands scaler would have...

• frontSize = 100
• backSize = 60
• frontY = 125
• backY = 65
• slopeNum = 100 - 60 = 40
• slopeDen = 125 - 65 = 60
• const = 60 - (100 * 65 / 60) = -48

Okay. frontY & backY are capping the scale. If a client moves lower/higher than those values, it won't get any bigger/smaller.

The problem hand earlier was at y:108.

(else (= temp1 (+ (/ (* slopeNum temp0) slopeDen) const)))

40 * 108 / 60 + -48 = 24

If the client's "scaleY" property isn't already 24...
(Debugger says a hands at that position has scaleY:113. Dunno where 113 came from.)

... the scaler will attempt to set scaleY to 24.

The assertion doesn't abort anything though.

"bp_write hands.3::scaleY" isn't stopping as the hand bobs up and down, so scaleY isn't constantly being set (or it is constantly set to 113?). Confused.

[Vhati]

Hrm.
I tried disposing the scaler and nulling the hand's "scaler" property.
Errors persisted.

I set the hand's "scaleX" and "scaleY" to 200.
Errors persisted.

The constant errors seem to begin the moment it submerges.
And they stop while it is still submerged, before it has risen.

Except sometimes it goes through an entire cycle without ANY errors!?


Hiding the last hand definitely makes the errors stop.

[Vhati]

"bp_write hands.3::scaleY" isn't stopping

I probably just can't set breakpoints on {nnnn:nnnn} or {name.n}.

I couldn't get the hand's cycler to trip until I set a breakpoint on the bare name.

[Vhati]

I disposed all the other hands and wisps.
The lone hand was only rarely throwing errors.

Went to restore and start over and... the control panel triggers an error!
If I show the control panel, specifically while the hand is submerged, there's a single error.

The assertion itself is failing during repaints, which don't happen unless the screen region is dirty. Of course, the painted object's properties could be set any time before that.


Prop::setScaler() was also setting the hands "scaleSignal" property to 1.
"send {hands} scaleSignal 0"
No more scaling (I'd manually set large scaleX/scaleY values earlier).
No more errors while submerged when the control panel is shown.

I toggled "scaleSignal" on and off, and I could cause the errors almost exclusively while it was on.
There was a little lag. I think the graphics needed to re-sync with the object before it really stopped scaling.

Weirdly, even scaling up is a problem (tried scaleX/scaleY = 400).

[sluicebox]

​https://github.com/scummvm/scummvm/pull/1710

[Filippos Karapetis <bluegr@…>]

In 2fd416b0:

SCI32: Extend scaler sanity checks to all SCI32 versions

Fixes QFG4 bug #10765. It's preferable to have sanity checks in the
code, rather than crashing due to invalid draw rectangles from buggy
game scripts. It's no use checking which specific interpreter versions
had sanity checks and trust the game scripts of the other interpreters.
Thus, it's easier and safer to always enable these sanity checks.

[bluegr]

Sanity checks have been put in place. Many thanks to everyone's work on this!
Closing