Opened 6 years ago

Closed 5 years ago

#10615 closed defect (fixed)

QFG4: Tentacle doesn't retract after crossing pit at start of game

Reported by: 2Mourty Owned by: bluegr
Priority: normal Component: Engine: SCI
Version: Keywords: SCI32 has-pull-request
Cc: Game: Quest for Glory 4

Description

ScummVM Version 2.1.0git2771-gca8b79fa75 (July 8th 2018)

Game Version DOS CD

Windows 7 32-bit

At beginning of the game when you cross the chasm in the dark one cave the Head Monk's tentacles come up and after the crossing they are supposed to go back down.

As of now the tentacles do not go down. The included save file is a mage. Use the cloth on-self and cast levitation spell.

Attachments (5)

qfg4-cd.003 (42.4 KB ) - added by 2Mourty 6 years ago.
sci.067 (40.7 KB ) - added by Vhati 5 years ago.
SavedGame (CD) - Stuck tentacle
sci.098 (40.0 KB ) - added by Vhati 5 years ago.
SavedGame (Floppy) - Stuck tentacle
sci.068 (41.8 KB ) - added by Vhati 5 years ago.
SavedGame (CD) - Stuck tentacle, fighter
sci.069 (41.5 KB ) - added by Vhati 5 years ago.
SavedGame (CD) - Stuck tentacle, rogue (tentacle does retract)

Download all attachments as: .zip

Change History (25)

by 2Mourty, 6 years ago

Attachment: qfg4-cd.003 added

comment:1 by Vhati, 5 years ago

I can't open the OP's savegame.

However, I can reproduce this if I max out the speed slider before attempting to cross.


ScummVM 2.1.0git3879-gb203b61b38 (Nov 13 2018 04:24:02)
Windows 7 64bit
QFG4 CD (English)

File - 5kb MD5 - Full MD5
RESOURCE.000 - 263dce4aa34c49d3ad29bec889007b1c - 1364ba69e3c0abb68cc0170650a56692
RESOURCE.AUD - c39521bffb1d8b19a57394866184a0ca - 71098b9e97e20c8941c0e4812d5f906f
RESOURCE.MAP - aba367f2102e81782d961b14fbe3d630 - 801a04cc6aa5d437681a2dd0b6545248
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

by Vhati, 5 years ago

Attachment: sci.067 added

SavedGame (CD) - Stuck tentacle

comment:2 by Vhati, 5 years ago

The CD's original interpreter also does this.

Occurs in the floppy edition under ScummVM, and its original interpreter, too.


ScummVM 2.1.0git3879-gb203b61b38 (Nov 13 2018 04:24:02)
Windows 7 64bit
QFG4 Floppy 1.1a + note patch (English)

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

by Vhati, 5 years ago

Attachment: sci.098 added

SavedGame (Floppy) - Stuck tentacle

comment:3 by Vhati, 5 years ago

sLevitateOverPit schedules and attempts to concurrently manage sTentacleDeath (the sequence presenting and retracting the tentacle). There is, of course, a race condition.


script 710 - sLevitateOverPit::changeState()

	0
	(g1_Glory handsOff:)
	(= register (if (> (g0_hero x?) 190) 1 else 0))

# hero is on the righthand side. register will be 1.
# ...
	5
	(if (not (proc0_4 101))
		(tentacle setScript: sTentacleDeath 0 1)
	)
# ...
	(if register
		(g0_hero setMotion: MoveTo 143 83 self)
# ...
	6
	(if (not (proc0_4 101)) ((tentacle script?) cue:))

After crossing, sLevitateOverPit advances sTentacleDeath to its next state, whatever that is, assuming cue() will be timed right to end it.


script 710 - sTentacleDeath

(0
	0
	(if (not register) (global1 handsOff:))
	(tentacle setLoop: 0 1 setCel: 0 show: setCycle: End self)
)
(1
	1
	(tentacle setLoop: 1 1 setCel: 0 setCycle: End self)
)
(2 2 (= cycles 6))
(3
	3
	(if register
		(= state 5)
		(tentacle setCycle: RandCycle tentacle)
	else
		(tentacle setLoop: 4 1 setCel: 0 setCycle: CT 3 1 self)
	)
)
(4
	4
	(global0 hide:)
	(torchEff hide:)
	(tentacle cycleSpeed: 10 setCycle: End self)
)
(5 5 (proc26_0 1 710 6))
(6
	6
	(tentacle setLoop: 0 1 setCel: 10 setCycle: Beg self)
)
(7
	7
	(tentacle hide:)
	(= register 0)
	(self dispose:)
)

Under normal circumstances sTentacleDeath goes through states 0, 1, 2, 3 (it sets state to skip ahead), 6 (retract), 7 (done).

State 3 sets a RandCycle. Incidentally, the original interpreter would have the tentacle wriggle, but it does not wriggle in ScummVM. With no goal of its own to trigger further progress, it waits for the external cue(). If the cue comes too soon, it'll hurry up and wait, indefinitely.

At max speed, the cue() comes so early it only advances to state 1. From there, it advances itself only up to 3.

When the tentacle is stuck like that, the debugger can unstick it: "send sTentacleDeath cue".

Last edited 5 years ago by Vhati (previous) (diff)

comment:4 by Vhati, 5 years ago

This affects fighters.

crossByHand::changeState()

The bug is latent in rogues because they're delayed with more intricate movement prior to their cue().

tightRope::changeState()

Last edited 5 years ago by Vhati (previous) (diff)

by Vhati, 5 years ago

Attachment: sci.068 added

SavedGame (CD) - Stuck tentacle, fighter

by Vhati, 5 years ago

Attachment: sci.069 added

SavedGame (CD) - Stuck tentacle, rogue (tentacle does retract)

comment:5 by Vhati, 5 years ago

Backtracking with the rogue is latent.

tightRopeLeft::changeState()

Backtracking with the fighter is latent.

crossByHandLeft::changeState()

  • Restore the fighter savegame
  • Cross from the right side, hand-over-hand. Tentacle sticks.
  • Unstick via the debugger: "send sTentacleDeath cue"
  • Cross from the left side, hand-over-hand. Tentacle retracts.

The mage isn't given an option to backtrack.
EDIT: Mages ought to have a way across. I must be forgetting how.

EDIT: Ah. I'd not played through as a mage. Walkthrough says - much later - they "Cast Calm. Levitate down into the pit. Take the book. Levitate back up on the other side of the pit." I think that involves sCalmDazzlePH and sHorrorSleeps.

The other classes descend via grapnel on the left, climb back up on the left, then cross the horizontal rope as they did at the start of the game.

EDIT: There's not even a giant tentacle to worry about later in the game. It's a white grub thing that needs to be defeated before crossing.

Last edited 5 years ago by Vhati (previous) (diff)

comment:6 by Vhati, 5 years ago

State 3 sets a RandCycle. Incidentally, the original interpreter would have the tentacle wriggle, but it does not wriggle in ScummVM.

Rather at slow speed with wizard, it wriggles a little and stops in ScummVM. There is a workaround acting on this.

Source: workarounds.cpp - arithmeticWorkarounds[]

	{ GID_QFG4,           710,64941,  0,          "RandCycle", "doit",                            NULL,     0,     0, { WORKAROUND_FAKE,   1 } }, // op_gt: when the tentacle appears in the third room of the caves

I couldn't identify the commit or bug it addressed.

Whatever the bug was, I didn't notice it when I commented out the workaround - CD or Floppy, high or low speed, multiple classes. Removing the workaround didn't restore the wriggle either. :/


EDIT: It was commit 259f262

Last edited 5 years ago by Vhati (previous) (diff)

comment:7 by Vhati, 5 years ago

One possible solution: slow down hero to make them lose the race.
Some scripts temporarily set hero's "moveSpeed" while crossing.

crossByHand, crossByHandLeft, and tightRope all set cycleSpeed & moveSpeed with a value from global433 (a constant, 6). tightRopeLeft calls Actor::setSpeed(), which is equivalent.

The others are already slow enough. Only crossByHand would benefit from slowing.

The debugger can fix crossByHand at max speed with "vv g 433 7" beforehand. A patch could use a literal 7 instead of the global. Downside: extra travel time costs a little more stamina. Not clear what mechanism drains stamina.

sLevitateOverPit does not set a speed. A patch would need to make room somehow.

comment:8 by Vhati, 5 years ago

If I understand the Script "cycles" property, it schedules a timed self-cue.

I can cram that in sLevitateOverPit::changeState(5) after it starts the tentacle and begins hero MoveTo. The next state (6) cues the tentacle.

crossByHand::changeState(2) is the last fighter MoveTo before 4 cues the tentacle. Nothing much stands out to optimize.


By itself "cycles" won't delay the traversal Scripts: hero's MoveTo is already self-cueing. If I decline to pass self to MoveTo in the first place, it won't cue(). I /think/ that might be safe.

Last edited 5 years ago by Vhati (previous) (diff)

comment:9 by Vhati, 5 years ago

The mage isn't given an option to backtrack.

The code used to be there.
"send hero setScript sLevitateOverPit" walks to the edge.

script 710 - sLevitateOverPit::changeState(0)

(= register (if (> (global0 x?) 190) 1 else 0))
(if register
	# Right stalagmite.
	(g0_hero setMotion: PolyPath 218 48 self)
else
	# Left stalagmite.
	(g0_hero setMotion: PolyPath 143 83 self)
)



However, hero's cloth action and levitate's effect are only allowed on the righthand side.

script 28 - hero::doVerb(54)

(and
	(== g11_myRoomNum 710)
	(> (g0_hero x?) 190)
	(< (g0_hero y?) 125)
)
(self setScript: (ScriptID 710 1)) # sLevitateOverPit
(return 1)

script 21 - levitateSpell::doVerb(4)

(and
	(== g11_myRoomNum 710)
	(== (g0_hero script?) (ScriptID 710 1)) # sLevitateOverPit
	(== ((g0_hero script?) state?) 3)
)
((g0_hero script?) register: 1)
((g0_hero script?) cue:)



Then register became a flag, not for the direction, but for just crossing at all.

script 710 - sLevitateOverPit::changeState()

(4
# ...
# Skip the crossing if register wasn't set.
	(if register (= cycles 1) else (self changeState: 8))
)
(5
# ...
	(if register
		# Go left.
		(g0_hero setMotion: MoveTo 143 83 self)
	else
		# Go right. This will never happen.
		(g0_hero setMotion: MoveTo 218 48 self)
	)
)
# ...
# State 7 skips over 8, and the two routes merge again at 9 to reset hero.
(8
	# "It's hard to do much with the wind pushing at your sheet,
	#   so you put it away."
	(g91_gloryMessager say: 10 6 8 0 self)
)

comment:10 by Vhati, 5 years ago

Not clear what mechanism drains stamina.

script 710 - crossByHand::doit()

(method (doit)
	(super doit: &rest)
	(if (== state 2) (g0_hero useStamina: 1))
)

comment:11 by Vhati, 5 years ago

Fixing each hero script in its own way is messy. If sTentacleDeath::changeState(0) could set a fast cycleSpeed, so it'd pop up faster, all of this might be fixed in one place.

Last edited 5 years ago by Vhati (previous) (diff)

comment:12 by Vhati, 5 years ago

Summary: QFG4: Head Monk tentacles do not go back down after crossing chasm at start of gameQFG4: Tentacle doesn't retract after crossing pit at start of game

comment:13 by Vhati, 5 years ago

a fast cycleSpeed, so it'd pop up faster, all of this might be fixed in one place.

A heap patch for cycleSpeed 4 or less does the trick.

A bit fast when hero crosses at sub-max speeds. Ideally, it would vary a little based on hero (maybe a multiplier with a floor/ceiling).

Enough of that for now. On to the wriggling problem...

comment:14 by Vhati, 5 years ago

The tentacle is given a RandCycle, which gets stuck.


script 710 - sTentacleDeath::changeState(3)

(tentacle setCycle: RandCycle tentacle)

It's not that RandCycle itself is broken because the torch flickering works.


script 710 - rm710::init()

(torchEff init: setScaler: global0 setCycle: RandCycle)

One obvious difference: the torch cycler has no arg.


setCycle() constructs a new cycler and calls the cycler's init(), prepending self to the args.


script 64998 - Prop::setCycle()

(method (setCycle param1)
	(if cycler (cycler dispose:))
	(if param1
		(= cycler
			(if (& (param1 -info-?) $8000)
				(param1 new:)
			else
				param1
			)
		)
		(cycler init: self &rest)
	else
		(= cycler 0)
	)
)

That means
"(tentacle setCycle: RandCycle tentacle)"
becomes
"(RandCycle init: tentacle tentacle)"


script 64941 - RandCycle::init()

(method (init param1 param2 param3 param4)
	(super init: param1)
	(if (>= argc 4) (= reset param4))
	(if reset (client cel: 0))
	(= cycleCnt (GetTime))
	(if (>= argc 2)
		(if (!= param2 -1)
			(= count (+ (GetTime) param2))
		else
			(= count -1)
		)
		(if (>= argc 3) (= caller param3))
	else
		(= count -1)
	)
)

This must be why someone added an arithmetic workaround.
RandCycle::doit() does a greater-than op with count.
The extra tentacle arg is being assigned to count!

Strange that ScummVM doesn't panic when init() attempts addition on the object.

comment:15 by Vhati, 5 years ago

A heap patch for cycleSpeed 4 or less does the trick.

A bit fast when hero crosses at sub-max speeds.
Ideally, it would vary a little based on hero

Patching crossByHand and sLevitateOverPit to use a fixed slow speed for traversal was surprisingly easy. Looks better than a fast tentacle.


To stamp out intermittency at all slider levels, I had to go slower than anticipated. I got misleading success when I teleported rather than playing through or loading a savedgame.

I remember one literal value that worked when the slider was at max, and min, but failed when the slider was somewhere in the middle. I'm guessing there's convoluted cycle counting going on.


Downside: extra travel time costs a little more stamina.

The mechanism behind the drain is already sapping 1 point at a time. It can't be reduced further, and the method is too short to modify.

The drain is noticeable (~80 points patched vs ~40 points unpatched, down from starting at ~225). However stamina loss isn't that big a deal in this game.

comment:16 by Vhati, 5 years ago

To recap

  • All the other scripts are fine.
  • Mage's sLevitateOverPit is solved by giving it a fixed speed.
  • Fighter's crossByHand originally had a fixed speed but drains stamina during the crossing, which makes setting a new slower speed problematic.



script 710 - crossByHand::doit()

(method (doit)
	(super doit: &rest)
	(if (== state 2) (g0_hero useStamina: 1))
)

Stamina only drains in state 2, the crossing itself!

If I delay in state 3, there's no drain.


script 710 - crossByHand::changeState()

(3
	(if (not (proc0_4 392))  # Never crossed before.
		(proc0_2 392)

		# "You just barely made it across the pit."
		(g91_gloryMessager say: 10 6 32 0 self)
	else
		(self cue:)
	)
)
(4
	(if (not (proc0_4 101)) ((tentacle script?) cue:))
# ...
)



By itself "cycles" won't delay [...] MoveTo is already cueing self. If I decline to pass self [as a caller arg], it won't cue().

Messing with MoveTo was a bad idea.
Nevertheless, the principle can be applied to say().

The code above advances in two different ways: a callback from say() or a self-cue().


Fighter Patch

(3
	(if (not (proc0_4 392))  # Never crossed before.
		(proc0_2 392)

		# Last arg is null instead of self.
		(g91_gloryMessager say: 10 6 32 0 0)

	# Use cycles to advance, not say() or a self-cue.
	(= cycles 32)
)
(4
	(if (not (proc0_4 101)) ((tentacle script?) cue:))
# ...
)

Fighter crosses as usual, draining the normal amount of stamina. There's a message at the end. Then hero hangs on the rope for a moment longer, cues tentacle, and dismounts.

Version 2, edited 5 years ago by Vhati (previous) (next) (diff)

comment:17 by Vhati, 5 years ago

Here's how the Mage script goes...

  • State 0
    • handsOff()
  • State 1-3
    • Cache the slider speed off hero.
    • Set a temporary speed for unfurling the cloth.
    • Restore the original slider speed.
    • handsOn()
  • State 4
    • handsOff()
    • Decide whether to cross (states 5-7) or abort (8).
  • State 5-7
    • Move across the pit, skip to state 9.
  • State 8
    • An abort message.
  • State 9-10
    • Both routes merge here.
    • Cache the slider speed off hero.
    • Set a temporary speed for folding up the cloth
    • Restore the original slider speed, and normalize hero.
    • handsOn()



script 710 - sLevitateOverPit::changeState()

(1
	1
	# Cache the original slider speed.
	(= local2 (g0_hero cycleSpeed?))

	(g0_hero
		view: 711
		loop: 0
		cel: 0
		cycleSpeed: 8  # Temporary fixed speed.
		setCycle: End self
	)
)

# ...

(3
	3
	# Restore the original slider speed.
	(g0_hero cycleSpeed: local2 setLoop: 1 1 setCycle: Fwd)
# ...
	(g1_Glory handsOn:)
)

(4
	4
	(g1_Glory handsOff:)
# ...
	# Cross if Levitate set register=1, or abort.
	(if register (= cycles 1) else (self changeState: 8))
)

(5
	5
# ...
	(if register  # Always 1.
		# Move left. Always.
		(g0_hero setMotion: MoveTo 143 83 self)
	else
		# Move right. Never happens.
		#   This is derelict code.
		(g0_hero setMotion: MoveTo 218 48 self)
	)
)

# ...

(9
	9
	# Cache original slider speed again.
	# Player could've adjusted the speed between
	#   state 3 (cloth deployed) and state 4 (Levitate).
	#
	(= local2 (g0_hero cycleSpeed?))

	(g0_hero
		view: 711
		setLoop: 0 1
		setCel: 2
		cycleSpeed: 8  # Temporary fixed speed.
		setCycle: Beg self
	)
)
(10
	10
	# Restore the original slider speed.
	(g0_hero cycleSpeed: local2 normalize: 1)
# ...
)



sLevitateOverPit already caches in state 1 and restores in state 10. That's good.

In the middle, a patch clobbers state 5's derelict code... caching again and setting a fixed speed that lasts throughout the crossing.

At state 9, we don't want the fixed speed cached, so that line is erased. This has to be a second patch because the CD and floppy editions have different byte counts.

That's it. State 10 restores the slider speed (whichever value was last cached).



Technically if there's an abort, the re-caching in state 5 is skipped, and the original value is restored.

Last edited 5 years ago by Vhati (previous) (diff)

comment:18 by Vhati, 5 years ago

Keywords: SCI32 has-pull-request added; Tentacle removed

Pull Request: SCI32: Fix QFG4 cave tentacle

comment:19 by Filippos Karapetis <bluegr@…>, 5 years ago

In aa9a1ab:

SCI32: Fix QFG4 cave tentacle

Fixes wriggling and retraction when hero travels over the pit, bug #10615
Supersedes commit 259f262

comment:20 by bluegr, 5 years ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.