Opened 10 months ago

Closed 4 months ago

#10835 closed defect (fixed)

QFG4: Mage stuck unable to cast spells in endgame crystal room

Reported by: Vhati Owned by: sluicebox
Priority: high Component: Engine: SCI
Keywords: SCI32 original Cc:
Game: Quest for Glory 4

Description

ScummVM 2.1.0git4278-gd31e37683c (Dec 13 2018 04:20:25)
Windows 7 64bit
QFG4 CD (English)

In the crystal room with Ad and Kat...

Mages need to tell the Ultimate Joke, cast Summon Staff, cast a projectile spell (killing Ad Avis), then cast Summon Staff again to complete the game.

Mages are unable to summon the staff a second time.

"You're too busy to cast a apell right now"
message pool 21 - noun:0 verb: cond:22

That's displayed from an asm block in script 21 - SpellItem::doVerb() at code_03ac, if g2_myCurrentRoom's "script" property is already set.

The original interpreter also does this.


Workaround: This does not happen if the speed slider is set to max before casting the projectile spell.


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

Attachments (2)

Disasm (CD) - Script 32, project changeState.txt (17.8 KB ) - added by Vhati 10 months ago.
Disasm (CD) - Script 730, avis getHurt.txt (3.1 KB ) - added by Vhati 10 months ago.

Download all attachments as: .zip

Change History (28)

comment:1 by Vhati, 10 months ago

Those specific actions are not a problem in the floppy edition under ScummVM.


QFG4 Floppy 1.1a + note patch (English)

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

comment:2 by Vhati, 10 months ago

Mages get stuck in BOTH the CD and floppy editions if they neglect to tell the Ultimate Joke.

Ad Avis, undistracted, shrugs off the attack and proceeds to taunt and gradually blast hero to death. Mages will be unable to cast spells. Max speed does not help.

comment:3 by Vhati, 10 months ago

Quick and dirty way to reproduce...

  • Create a new mage character.
  • Set flag 450 (necessary for the staff summoning; can't find where it's naturally set)
    • vv g 528 8192
  • Learn "Summon Staff" spell.
    • send hero learn 32 200
  • Teleport to the endgame crystal room.
    • room 730
  • Esc through the dialogue.
  • You'll have a chance to save as hero climbs up to stand.

Win route (CD)

  • Click MOUTH on hero, and "Tell Ultimate Joke".
  • Ad Avis will become distracted.
  • Bring up the spell list (star icon), and cast "Summon Staff" (3rd row, 3rd icon).
  • Cast "Force Bolt" at Ad Avis (2nd row, 4th icon).
  • Dismiss the Force Bolt cursor.

After Ad Avis is killed, you're supposed to summon the staff again, but you're told hero is too busy.

"vo rm730 script" reports the room is stuck doing project::changeState(2) from script 32.

Mistake route (CD/Floppy)

  • No joke.
  • Bring up the spell list (star icon), and cast "Summon Staff" (3rd row, 3rd icon).
  • Cast "Force Bolt" at Ad Avis (2nd row, 4th icon).

Ad Avis shrugs it off and blasts hero.

"vo rm730 script" reports the room is doing sMessages::changeState(3) from script 730.

Last edited 10 months ago by Vhati (previous) (diff)

comment:4 by Vhati, 10 months ago

SCI Companion got confused, so I decompiled project by hand.
I might've made mistakes. Raw disasm from ScummVM is attached.


script 32 - project::changeState()

(switch (= state param1))
	(0
		(proc0_12 g70 (- g71 10))
		(g1_Glory handsOff:)

		# Turn hero to face toward the clicked point,
		#   set by doVerb() methods in script 730.
		(proc0_10 g0_hero g441_myX g442_myY self)
	)
	(1
		# Treat theSword as theThrowDagger!?
		#   theSword should not even send us here.
		(if (== register_verb 36)
			(= register_verb 37)
		)
		(= loc3_heading (g0_hero heading:))
		(= loc0 (g0_hero loop:))
		(g0_hero setMotion: 0)

		(g0_hero
			view:
				(cond
					# Mundane thrown projectile.
					((< register_verb 75)
						9
					)
					# hero's holding the staff.
					((and (> (hero view:) 17) (< (hero view:) 21))
						18
					)
					(else
						14
					)
				)
			loop:
				(cond
					((and (<= 0 loc3_heading) (<= loc3_heading 85))
						(= loc2 2)
					)
					((and (<= 86 loc3_heading) (<= loc3_heading 180))
						(= loc2 0)
					)
					((and (<= 181 loc3_heading) (<= loc3_heading 274))
						(= loc2 1)
					)
					(else
						(= loc2 3)
					)
				)
			setCel: 0
		)

		(cond
			# Mundane thrown projectile.
			((< register_verb 75)
				(g0_hero setCycle: CT 4 1 self)
			)
			# hero's holding the staff.
			((and (> (g0_hero view:) 17) (< (g0_hero view:) 21))
				(g0_hero setCycle: CT 2 1 self)
			)
			(else
				(g0_hero setCycle: End self)
			)
		)
	)
	(2
		# Mundane thrown projectile.
		(if (proc64999_5 register_verb 21 37)
			# 10 is g257_myThrowing.
			(if [g247 10]
				(g0_hero
					useSkill: 10 25  # g257_myThrowing
					useSkill: 0 3    # g247_myStrength
					useSkill: 2 5    # g249_myAgility
				)
			)
			(g0_hero useStamina: 2)
			(= temp0 (Random 0 (>> (+ (- 400 [g247 10]) 4) 2)))
			(= g441_myX (+ g441_myX (if (Random 0 1) (- 0 temp0) else temp0)))
		)

		((ProjObj new:) fixPriority: 1 type: register_verb init:)

		(switch register_verb
			(86 (spellSoundFX number: 933 play:))
			(88 (spellSoundFX number: 938 play:))
			(93 (spellSoundFX number: 974 play:))
			(79 (spellSoundFX number: 983 play:))
			(else (spellSoundFX number: 916 play:))
		)
		(if
			(or
				# hero's holding the staff.
				(and (> (g0_hero view:) 17) (< (g0_hero view:) 21))
				# Mundane thrown projectile.
				(< register_verb 75)
			)
			(g0_hero setCycle End self)
		else
			(g0_hero setCycle Beg self)
		)
	)
	(3
		(if (not (and (> (g0_hero view:) 17) (< (g0_hero view:) 21)))
			# hero's NOT holding the staff.
			(g0_hero normalize: 0)
		else
			(g0_hero
				view: 20
				loop: (loc11 at: (g0_hero loop:))
				cel: (if (< (loc11 at: (g0_hero loop:)) 6) 4 else 5)
			)
		)
		(= cycles 2)
	)
	(4
		(if (not (proc0_4 9))
			(g1_Glory handsOn:)
		)
		(self dispose:)
	)
)



Stepping through at regular and max speed then diffing the logs showed state 2 is playing out exactly the same, regardless of speed. Except for some reason, it can reach state 3 at max. At slower speeds, it cannot.

Last edited 10 months ago by Vhati (previous) (diff)

comment:5 by Vhati, 10 months ago

Another decompilation by hand.
Raw disasm from ScummVM is attached.


script 730 - avis::getHurt()

(if (and (not loc10_heroDied) (not loc2_avisDied))
	(cond
		(
			(and
				loc4_toldJoke
				(not (g0_hero script:))
				(!= loc1 0)
			)
			(if (not (and (> (g0_hero view:) 17) (< (g0_hero view:) 21)))
				# hero's NOT holding the staff.
				(g2_myCurrentRoom setScript: sMessages)
			)
		)
		((and loc4_toldJoke (not (g0_hero script:)))
			(g0_hero view: 5 setLoop: 5 1)
			(g0_hero setScript: sAdavisDies)
		)
		(else
			(g2_myCurrentRoom setScript: sMessages)
		)
	)
)



The following came from SCI Companion, with variables renamed for clarity.


script 730 - sMessages::changeState()

(switch (= state param1)
	(0
		(g1_Glory handsOff:)
		(= ticks 60)
	)
	(1
		(if loc4_toldJoke
			# "Ad Avis may be helpless at the moment, but that
			#   doesn't mean his protective spells aren't working.
			#   You'll need to find a more powerful attack to do
			#   him any serious damage."
			(g91_gloryMessager say: 6 6 9 0 self)
		else
			(= ticks 1)
		)
	)
	(2
		(if (== loc1 0)
			# "You think I haven't arranged protections against
			#   all of your spells? I know what puny magics the
			#   Wizards teach such as you!"
			(g91_gloryMessager say: 5 6 5 0 self)
		else
			# "Ha! There are advantages to being a Vampire. Those
			#   puny mundane weapons can do nothing to me!"
			(g91_gloryMessager say: 5 6 10 0 self)
		)
	)
	(3
		(self setScript: sCastASpell)
	)
)



script 730 - sCastASpell::changeState()

(switch (= state param1)
	(0
		(g1_Glory handsOff:)
		(avis
			view: 677
			setLoop: 0 1
			setCel: 0
			setCycle: CT 5 1 self
		)
	)
	(1
		((= loc0_fireBall (fireBall new:))
			view: 747
			x: 91
			y: 101
			setLoop: 1 1
			moveSpeed: 0
			init:
			setMotion: MoveTo 176 60 self
		)
		(avis setCycle: End)
	)
	(2 (loc0_fireBall setCycle: End self))
	(3
		(loc0_fireBall dispose:)
		(cond 
			# hero's holding the staff.
			(
				(and
					(> (g0_hero view?) 17)
					(< (g0_hero view?) 21)
					(< (g0_hero view?) 21)
				)
				(g0_hero takeDamage: 23)
			)
			# ???
			((> g454 0)

				# [g247 26] refers to g273_myFlameDartSkill.
				(g0_hero
					takeDamage: (- 50 (/ (* [g247 26] 15) 100))
				)
			)
			(else (g0_hero takeDamage: 50))
		)

		# [g247 17] refers to g264_myHealth.
		(if (== [g247 17] 0)
			(self setScript: sEgoDies)
		else
			(self cue:)
		)
	)
	(4
		# Random message:
		#   "Let's see how well you can protect yourself from this spell!"
		#   "Hot enough for you?"
		#   "I've long had a burning desire to do this to you!"
		#   "I wonder how many more spells it will take to destroy you
		#     utterly. Shall we find out?"
		(proc0_17 5 6 13 sCastASpell 730)
	)
	(5
		(g1_Glory handsOn:)
		(g69_mainIconBar disable: 0)
		(self dispose:)
	)
)
Last edited 10 months ago by Vhati (previous) (diff)

comment:6 by Vhati, 10 months ago

project (ScriptID 32) is triggered in a lot of ways: every spell projectile, rocks, and throwing dagger.


script 730 - rm730::doVerb()

# Does not set g441_myX and g442_myY.
# ...
(86
	# 26 is g273_myFlameDartSkill.
	(g0_hero trySkill: 26)
	(= loc1 4)
	(self setScript: (ScriptID 32) self 86)
)
(88
	# 28 is g275_myForceBoltSkill.
	(g0_hero trySkill: 28)
	(= loc1 5)
	(self setScript: (ScriptID 32) self 88)
)
(79
	# 34 is g281_myFrostbiteSkill.
	(g0_hero trySkill: 34)
	(= loc1 6)
	(self setScript: (ScriptID 32) self 79)
)
# ...
(93
	# 33 is g280_myLightningBallSkill.
	(g0_hero trySkill: 33)
	(= loc1 8)
	(self setScript: (ScriptID 32) self 93)
)
# ...
(92
	(if (== (g0_hero view?) 20)
		# "You've already summoned the staff."
		(g91_gloryMessager say: 0 92 38)
		(return 1)
	else
		(= loc1 9)
		# script 46 - staffScript
		(self setScript: (ScriptID 46) self)
	)
)
# ...

script 730 - rm730::cue()

(switch loc1
	(9
		(if loc2_avisDied
			(g2_myCurrentRoom setScript: sDoTheEndGame)
		else
			(g2_myCurrentRoom setScript: sSummon)
		)
	)
	# Your spell disappears into the distance having had no useful effect.
	(4 (global91 say: 0 86 0 0))
	# These are all blank.
	(5 (global91 say: 0 88 0 0))
	(6 (global91 say: 0 79 0 0))
	(8 (global91 say: 0 93 0 0))
)
(= loc1 0)



script 730 - avis::doVerb()

# Informs project.
(= g441_myX ((User curEvent?) x?))
(= g442_myY ((User curEvent?) y?))
# ...
(21
	# 10 is g257_myThrowing.
	(g0_hero trySkill: 10)
	(= loc1 0)
	# 6 is theRocks item,
	#   available via debugger with "send hero get 6".
	(g0_hero use: 6)
	(g2_myCurrentRoom setScript: (ScriptID 32) 0 21)
)
(37
	(if (== (g0_hero has: 5) 1)
		# "You are down to your last dagger.
		#   You'd better hold on to it."
		(g91_gloryMessager say: 6 6 40)
	else
		# 10 is g257_myThrowing.
		(g0_hero trySkill: 10)

		# 5 is theThrowdagger item,
		#   available via debugger with "send hero get 5".
		(g0_hero use: 5)
		(= loc1 0)
		(g2_myCurrentRoom setScript: (ScriptID 32) 0 37)
	)
)
(86
	# 26 is g273_myFlameDartSkill.
	(g0_hero trySkill: 26)
	(= loc1 4)
	(g2_myCurrentRoom setScript: (ScriptID 32) 0 86)
)
(88
	# 28 is g275_myForceBoltSkill.
	(g0_hero trySkill: 28)
	(= loc1 5)
	(g2_myCurrentRoom setScript: (ScriptID 32) 0 88)
)
(93
	# 33 is g280_myLightningBallSkill.
	(g0_hero trySkill: 33)
	(= loc1 8)
	(g2_myCurrentRoom setScript: (ScriptID 32) 0 93)
)
(79
	# 34 is g281_myFrostbiteSkill.
	(g0_hero trySkill: 34)
	(= loc1 6)
	(g2_myCurrentRoom setScript: (ScriptID 32) 0 79)
)
# ...



script 730 - crystal::doVerb()

(if loc2_avisDied
	# Informs project.
	(= g441_myX ((User curEvent?) x?))
	(= g442_myY ((User curEvent?) y?))

	(switch param1
# ...
		# theThrowDagger item.
		(37
			(if (== (g0_hero has: 5) 1)
				# "You are down to your last dagger.
				#   You'd better hold on to it."
				(g91_gloryMessager say: 6 6 40)
			else
				# Blank.
				(g91_gloryMessager say: 1 37 0 0)
			)
		)
# ...
		# Flame Dart
		(86
			(self setScript: (ScriptID 32) crystal 86)
		)
		# Force Bolt
		(88
			(self setScript: (ScriptID 32) crystal 88)
		)
		# Frostbite
		(79
			(self setScript: (ScriptID 32) crystal 79)
		)
# ...
		# Lightning Ball
		(93
			(self setScript: (ScriptID 32) crystal 93)
		)
# ...
		# theStaff item, given to non-mage heroes,
		#   available via debugger with ("send hero get 47")
		(157
			(g2_myCurrentRoom setScript: sDoTheEndGame)
		)
# ...
	)
else
	# "You don't have time to deal with the crystal right now
	#   -- Ad Avis is trying to kill you!"
	(g91_gloryMessager say: 1 157 0 0)
)

script 730 - crystal::cue()

# "The crystal is totally unaffected by your spell."
(g91_gloryMessager say: 1 86 0 0)



script 32 - project::init()

(method (init p1_client p2_caller p3_verb param4)
	(= loc6 (IntArray with: 20 -20 10 -10))
	(= loc7 (IntArray with: 0 0 0 0))
	(= loc8 (IntArray with: 10 -10 -10 10))
	(= loc9 (IntArray with: -5 -5 -5 -5))
	(= loc10 (IntArray with: 0 1 0 0 0 1 0 1))
	(= loc11 (IntArray with: 2 3 6 7))
	(= lastTicks g88)
	(if (>= argc 1)
		((= client p1_client) script: self)
		(if (>= argc 2)
			(= caller p2_caller)
			(if (>= argc 3)
				(= register_verb p3_verb)
				(if (>= argc 4) (= loc4 param4) else (= loc4 0))
			)
		)
	)
	(= state (- start 1))
	(self cue:)
)
Last edited 10 months ago by Vhati (previous) (diff)

comment:7 by Vhati, 10 months ago

Possibly related: Ad Avis continues blasting even when the game is ostensibly paused. hero will die if the dialogue menu or the spell list is left up for too long.

Last edited 10 months ago by Vhati (previous) (diff)

comment:8 by Vhati, 10 months ago

Workaround: Only freshly cast spells are aborted due to an existing room script. After killing Ad Avis, if instead of dismissing the lingering Force Bolt cursor, you shoot again, at the room, the second projectile will take over as room script. That one WILL completely advance, dispose itself, and allow further casting.



Another error: If, after killing Ad Avis, you use the lingering Force Bolt cursor to shoot the crystal... Then shoot the room...

parameter 0: 0000:0004 (integer), should be null, integer
parameter 1: 000f:0006 (reference, invalid), may be any (optional) (more may follow)
[VM] kArray[82]: signature mismatch in method IntArray::callKernel (room 730, script 64920, localCall ffffffff)!

rm730::setScript() is calling project::dispose().
It chokes at pc=0199, the op after local6::dispose().


script 32 - project::dispose()

(method (dispose)
	(local6 dispose:)
	(local7 dispose:)
	(local8 dispose:)
	(local9 dispose:)
	(local10 dispose:)
	(local11 dispose:)
	(super dispose: &rest)
)
  • After killing Ad Avis, rm730's script is project::changeState(2). Dunno why.
  • Attempting to shoot the crystal calls crystal::setScript(), not the room's. rm730 still has a mysteriously not-removed reference to project from shooting Ad Avis.
  • After shooting the crystal, the script will advance, dispose() itself properly, and crystal's reference will be null. rm730's lingering reference will report state 4. The crystal can be shot repeatedly.
  • Attempting to shoot the room has rm730::setScript() dispose the lingering already-disposed room reference. Which is what the array error meant.



Shooting the room (clears the room script, advances, and removes itself properly), then the crystal, then the room... no error.

Last edited 10 months ago by Vhati (previous) (diff)

comment:9 by Vhati, 10 months ago

One problem identified. There's only one instance of project getting recycled. All setScript() calls should be on the same object to avoid zombie references.

Last edited 10 months ago by Vhati (previous) (diff)

comment:10 by Vhati, 10 months ago

In project state 2, hero has view:18, holding the staff. Cels 0-9.

An End cycler is set on hero, which completes and cues when the endCel it saw on hero's view during init() is reached.


project::changeState(2)

(g0_hero setCycle End self)



avis' getHurt() is switching hero to view:5, plain standing. Only a cel 0.


avis::getHurt() [1c07]

((and loc4_toldJoke (not (g0_hero script:)))
	(g0_hero view: 5 setLoop: 5 1)
	(g0_hero setScript: sAdavisDies)
)

hero's setLoop() updates hero's cycler's clientLastCel.


script 64998 - Prop::setLoop()

(method (setLoop &tmp temp0)
	(= temp0 loop)
	(super setLoop: &rest)
	(if (and cycler (!= temp0 loop))
		(cycler clientLastCel: (self lastCel:))
	)
)

But the cycler's endCel was NOT updated.


script 64992 - CT::doit()

(method (doit &tmp temp0 temp1)
	(if
	(!= (= temp1 (client cel?)) (= temp0 (self nextCel:)))
		(if (== temp1 endCel)
			(self cycleDone:)
		else
			(client cel: temp0)
		)
	)
)



Assuming some mechanism disallows values beyond the loop's last cel, hero's cycler will never reach the index it expects. Never complete. And never cue.

At max speed, hero's cycler not only completes... project advances all the way to disposal... before ProjObj::doit() detects collision with Ad Avis, triggering getHurt() to replace hero's view.

Last edited 10 months ago by Vhati (previous) (diff)

comment:11 by Vhati, 10 months ago

Another problem identified. ProjObj needs to be slowed to allow project time to advance and dispose.

comment:12 by Vhati, 10 months ago

In the floppy edition, getHurt() also clobbers hero's view.
It has an else instead of a cond for sMessages.


Floppy, Script 730 - avis::getHurt()

(if (and (not loc10_heroDied) (not loc2_avisDied))
	(if (and loc4_toldJoke (not (g0_hero script?)))
		(g0_hero view: 5 setLoop: 5 1)
		(g0_hero setScript: sAdavisDies)
	else
		(g2_myCurrentRoom setScript: sMessages)
	)
)



Aside from innocuous hex variations, floppy's project::changeState() is almost identical.
Only one irrelevant line differs.

Floppy: "(g0_hero useStamina: 1)"
CD: "(g0_hero useStamina: 2)"


More significantly, in script 64998, the floppy edition Prop class had no setLoop() method.
It doesn't do anything to the cycler.

The superclass View::setLoop() only sets the "loop" property (and some signal stuff that doesn't apply here).
Maybe without CD's incomplete meddling the cycler doesn't stall?


In any case, getHurt() should not be allowed to replace hero's view before project finishes. Slowing ProjObj will be good for both editions.

Last edited 10 months ago by Vhati (previous) (diff)

comment:13 by Vhati, 10 months ago

sMessages hangs around because it lacks a final state that disposes itself.
It does not loop. Several different scripts happen to cause fireBalls.


hero will die if the dialogue menu or the spell list is left up for too long.

rm730::init() sets the room script to enterScr, which does the cutscene then calls crystal::setScript(sTimeItOut) when it's done.


script 731 - enterScr

((ScriptID 730 9) setScript: (ScriptID 730 10))

sTimeItOut quietly counts down seconds then makes Ad Avis launch an instakill fireBall.

sTimeItOut doesn't dispose itself either, or check if Ad Avis is alive. It just calls avis::setCycle(CT 5 1 self) and never gets a cue() to advance if he's already gone.

comment:14 by Vhati, 10 months ago

Grr.

Had to slow ProjObj a LOT. (setStep 3 1)
Increasing moveSpeed from the original 0 didn't help.

Or had to speed up hero a LOT. ("send hero cycleSpeed 2")
That would've been a tricky patch anyway.


Maybe the view replacement could be moved from getHurt() later, into sAdavisDies somewhere?
I don't see easy redundancy there.


Or getHurt() could take its interruption further: remove project with "((ScriptID 32) dispose:)". No room for that in floppy edition, but it wasn't symptomatic.

Last edited 10 months ago by Vhati (previous) (diff)

comment:15 by sluicebox, 10 months ago

WOW this is Gilgamesh level bug reporting! Thanks for posting your heroic manual decompilations, that really speeds things up.

I'm in the process of testing a patch set for these bugs and #10844. There's a good chance I'll find more edge cases in this room and have to adjust further, plus I need to test with other hero types, but it's looking good so far.

comment:16 by Vhati, 10 months ago

@sluicebox:

I'm in the process of testing a patch set for these bugs

Me too. Been meaning to summarize my status...

  • I moved project off crystal and onto the room. No more zombie "script" reference from being scheduled sometimes on crystal, sometimes on the room.
  • I moved sMessages off the room and onto avis. No more spell blocking. And its lack of self-disposal doesn't matter there.
  • Regarding ProjObj's travel time...
    • Some mistake that negated my attempts at setting moveSpeed threw me off initially, hence the setStep(). Whatever it was, movespeed and setSpeed() are cooperating now.
    • A single fixed value doesn't fit all slider positions well. I'm working on patching ProjObj::init() to vary moveSpeed's value based on hero's speed, with a bit of arithmetic to convert to the needed range of values. Need to verify that every value for speeds 0-15 will come out appropriate.
Last edited 10 months ago by Vhati (previous) (diff)

comment:17 by sluicebox, 10 months ago

That's funny, I'm juggling script ownership too.

What do you mean by moving project off crystal and onto the room? Do you have that backwards? project is already the room's script (not counting casting a spell at the crystal since that only works after avis is dead), so did you change it so that project is crystal's script? If so, won't that dispose crystal's existing script sTimeItOut and prevent it from expiring?

Won't setting sMessages as avis' script dispose sUltimateJoke if it's already started and prevent it form expiring too?

I assigned sMessages to midBlast as it's a Prop that's always initialized and doesn't have any scripts to conflict with. I changed sAdavisDies from hero's script to the room's script to force project's disposal even if it got interrupted with the view/loop change in avis:getHurt. For the death animation I just added a local2 test to sTimeItOut state 1.

The last detail I'd like taken care of is the holes that that his create between hands off and hands on. There was already one, but reassigning things so that project can complete while sMessages runs creates others, and you can the spell cursor when you shouldn't and can shoot multiple projectiles at once if you click fast. Without patches, doing this crashes reliably with the Grcycler null from other bug reports. I'd like to get it in hands off from the moment you throw/cast a projectile until all the work is done (getHurt aborting or sCastMessages completing), but that's a hard detail.

This is an impressively broken room! Cast a spell into nowhere or switch to sneak/run to go walk around in the air

comment:18 by Vhati, 10 months ago

@sluicebox:

What do you mean by moving project off crystal and onto the room?



From comments 8 and 9...

Another error: If, after killing Ad Avis, you use the lingering Force Bolt cursor to shoot the crystal... Then shoot the room...
[...]
There's only one instance of project getting recycled. All setScript() calls should be on the same object to avoid zombie references.

On reflection, I'd written that patch before tackling the ProjObj timing. project was getting stuck at the time, lingering on the room. After shooting crystal, it was throwing an exception, with references to the same object on crystal and the room causing an already disposed object to be disposed again.

When given time to complete, project disposes itself properly. So in theory, a good ProjObj patch would ensure project is off the room before it'd appear on crystal. It wouldn't matter that it's sometimes scheduled in different places if they're not simultaneous. So I wouldn't need that patch.


@sluicebox:

Won't setting sMessages as avis' script dispose sUltimateJoke

Running ScummVM's master branch... If you blast Ad Avis ineffectively (no staff) while he's laughing at the joke, he will stop laughing and sMessages takes control of his sprite. Both scripts do run concurrently in different places, although sUltimateJoke is idling until its insta-kill.


and prevent [sUltimateJoke] form expiring too?

sUltimateJoke's countdown never has time to expire in the CD edition. It only had a chance to trigger its insta-kill in the floppy because it was competing against a separate ridiculously long timer, which was arguably erroneous since the CD edition shortened that.

IMO sUltimateJoke's timer shouldn't be a concern. I'd even remove it except writing a patch to do would be an unnecessary effort.


@sluicebox:

I changed sAdavisDies from hero's script to the room's script to force project's disposal even if it got interrupted

I came to the conclusion that project needs to end naturally.
Better than dealing with the fallout from an interruption, as you've witnessed.


reassigning things so that project can complete while sMessages runs creates others

I don't see sMessages as that big a deal. What it does is brief. It doesn't clean up after itself when it completes. It just shouldn't be on the room. midBlast or avis is fine.


For the death animation I just added a local2 test to sTimeItOut state 1.

I rather like what I came up with for #10844 (reducing literals to a formula to make room for the local2 check).

I'm not especially enthusiastic about varying ProjObj speed. I mean I think it'll work, but the way I made room there (reducing y:'s (g0_hero view?) switch to a simple if-else) made the patch longer than I'd like.

A fixed hero cycleSpeed in project would be nice (probably in tandem with a new fixed ProjObj speed), but I didn't see room at the end of project to restore hero's original value.

Last edited 10 months ago by Vhati (previous) (diff)

comment:19 by Vhati, 10 months ago

@sluicebox:

shoot multiple projectiles at once if you click fast

Hm. Multiple projectiles might be an edge with a ProjObj speed patch, too... although a narrower window since I'm aiming to match ProjObjs's travel time with project's life cycle.

comment:20 by Vhati, 10 months ago

@sluicebox:

Cast a spell into nowhere or switch to sneak/run to go walk around in the air

OMG

  • They didn't pen hero in with a poly.
  • They just disable the FOOT action after -almost- every g1_Glory::handsOn() (which enables all actions)... except the handsOn() in project::changeState(4).
  • They don't disable the run/sneak gaits.
Last edited 10 months ago by Vhati (previous) (diff)

comment:21 by Vhati, 10 months ago

Ohhh... oh no.
I was investigating the flag (9) that projectile checks.
It's the only place in the game that checks it.

(4
	(if (not (proc0_4 9))
		(g1_Glory handsOn:)
	)
	(self dispose:)
)

That flag is never set anywhere in this room. Guess what.
Apparently 580, 740, and 800 set flag 9 to communicate a desire to keep handsOff after projectile finishes. They unset the flag later, even as late as the room's dispose().

projectile/ProjObj (ScriptID 32) are used by lots of other rooms.
10, 50, 51, 360, 480, 535, 579, 580, 600, 625, 632, 680, 710, 730, 740, 760, 770, 800

Something to keep in mind when considering alterations.


I can restrict the scope of my moveSpeed patch by wrapping it in a room number check.

Last edited 10 months ago by Vhati (previous) (diff)

comment:22 by Vhati, 9 months ago

@sluicebox:

Cast a spell into nowhere or switch to sneak/run to go walk around in the air

I've got a poly added with a couple more patches.

comment:23 by Vhati, 9 months ago

Last edited 9 months ago by Vhati (previous) (diff)

comment:25 by Filippos Karapetis <bluegr@…>, 4 months ago

In a486438:

SCI32: Fix QFG4 Ad Avis end-game bugs

Fixes bugs #10835, #10844, #10989

comment:26 by bluegr, 4 months ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.