Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10860 closed defect (fixed)

QFG4: Lock up when casting Trigger after Summon Staff

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

Description

Casting trigger on the gate never ceases.

Attachments (3)

qfg4-cd-gog.020 (92.4 KB ) - added by tomasz89 6 years ago.
Disasm (CD) - Script 11, castTriggerScript changeState.txt (14.0 KB ) - added by Vhati 6 years ago.
Decomp (CD) - Script 13, castOpenScript changeState.txt (2.5 KB ) - added by Vhati 6 years ago.

Download all attachments as: .zip

Change History (20)

by tomasz89, 6 years ago

Attachment: qfg4-cd-gog.020 added

comment:1 by Vhati, 6 years ago

@tomasz89:
Can you elaborate?
I could not reproduce with my CD or floppy editions under ScummVM.

I don't think GOG ships any patches to the gate room (script 600) or Trigger (script 11)

Here's what I did.
Do you get the bug if you do the same?

  • Create a new mage character.
  • Bring up the debugger (ctrl-shift-d to show, escape to hide).
    • Acquire the Summon Staff spell.
      • send hero learn 32
    • Teleport to the gate.
      • room 600
  • Cast Summon Staff
  • Cast Trigger on the gate.

For me, the narrator says...

  • "Your magical staff fails to appear; you are too far from where you created it. Perhaps you can find a new one here."
  • "Your spell dissipates as if blocked by magic. It does not seem to have had any effect."

I tried casting both spells during the day and at night.


QFG4 CD (English)

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

QFG4 Floppy 1.1a + note patch (English)

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

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

comment:2 by tomasz89, 6 years ago

I get the same as you without summon staff first. If you summon staff then trigger, cactus. Same at Erana's garden where you attempt to get protection.

comment:3 by tomasz89, 6 years ago

I will retry as you have it though

Last edited 6 years ago by tomasz89 (previous) (diff)

comment:4 by tomasz89, 6 years ago

OK the difference is the summon staff fails because you don't possess the staff, so it's the same as casting trigger without the staff. In other words, you need to succeed in summoning the staff before this will trigger the condition.

Last edited 6 years ago by tomasz89 (previous) (diff)

comment:5 by tomasz89, 6 years ago

I took the sequence from #10835 and can reproduce it.

Create a new mage character.
Set flag 450

vv g 528 8192

Learn "Summon Staff" spell.

send hero learn 32 200

Teleport to the castle gate room.

room 600

Then cast summon staff, followed by trigger on the gate.

comment:6 by Vhati, 6 years ago

I see.

I CAN reproduce this in both my CD and floppy editions under ScummVM and their original interpreters.

Didn't have a lategame mage to test with, so I used Sierra's debug scripts. (alt-g 450, alt-k 32 200)


castTriggerScript is getting stuck in state 2.

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

comment:7 by Vhati, 6 years ago

Keywords: SCI32 original added
Summary: QFG4 summon staff + trigger lock upQFG4: Lock up when casting Trigger after Summon Staff

comment:8 by Vhati, 6 years ago

This bug can be seen at the squid monolith (room 800), and presumably any other triggerable location, when the staff is successfully summoned first.

comment:9 by Vhati, 6 years ago

SCI Companion got confused, so I decompiled by hand.
Raw disasm from ScummVM is attached.


script 11 - castTriggerScript::changeState()

(switch (= state param1)
	(0
		(= local4 (IntArray with: 0 0 0 1 0 0 2 3))
		(= local5 (IntArray with: 2 3 6 7))
		(= local6 (IntArray with: 2 3 0 3 0 1 2 3))
		(g1_Glory handsOff:)
		(g0_hero setHeading: (GetAngle (g0_hero x?) (g0_hero y?) g441_myX g442_myY) self)
	)
	(1
		(= local1 (go_hero loop?))
		(if (and (> (go_hero view?) 17) (< (go_hero view?) 21))
			# hero's holding the staff.
			(= local2 (go_hero cel?))
			(g0_hero
				view: 19
				loop: (local4 at: local1)
				setCel: 0
				setCycle: End self
			)
		else
			(g0_hero
				view: 14
				loop: (local6 at: local1)
				setCel: 0
				setCycle End self
			)
		)
	)
	(2
		(triggerEffect
			x: g441_myX
			y: g442_myY
			setScaler: g0_hero
			cycleSpeed: 0
			setPri: 180
			init:
			setCycle: Fwd
		)
		(if register
			(if (register onMe: g441_myX g442_myY)
				(= local0 register)
			)
			(= register 0)
		)
		(if (not (and (> (go_hero view?) 17) (< (go_hero view?) 21)))
			# hero's NOT holding the staff.
			(g0_hero setCycle: Beg self)
		)
# No else!?
		(soundFX number: 934 play:)
	)
	(3
		(triggerEffect dispose:)
		(= cycles 2)
	)
	(4
		(if (and (> (go_hero view?) 17) (< (go_hero view?) 21))
			# hero's holding the staff.
			(g0_hero
				view: 20
				loop: (local5 at: (g0_hero loop?))
				cel: (if (< (g0_hero loop?) 6) 4 else 5)
			)
		else
			(g0_hero normalize:)
		)

		(cond
			((not (proc64999_5 global11 270 290 340 440 460 520 580 593 600 641 643 650 750 800))
				# "Nothing seems to have been triggered."
				(g91_gloryMessager say: 0 0 1 0 self 11)
			)
			(local0
				(local0 doVerb: -82)  # 0xae
			)
			(else
				# "Nothing seems to have been triggered."
				(g91_gloryMessager say: 0 0 1 0 self 11)
			)
		)
	)
	(5
		(g1_Glory handsOn:)
		(self dispose:)
	)
)

It doesn't seem to have any means to advance if hero IS holding the staff.



EDIT: State 4's normalize is only when there's no staff.
EDIT: doVerb() is passed a signed byte, so 0xae is -82, not 174.

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

comment:10 by Vhati, 6 years ago

I tried some other spells and found that the Open spell has a similar effect without getting stuck.

I had to manually decompile that too, and it is indeed VERY similar.
It's attached. I'll quote the important difference.


script 13 - castOpenScript::changeState(2)

(2
# ...
	(if (not (and (> (go_hero view?) 17) (< (go_hero view?) 21)))
		# hero's NOT holding the staff.
		(g0_hero setCycle: Beg)

		# Unlike castTriggerScript, there's self arg to cue.
	)
	(soundFX number: 934 play:)

	# Use seconds to cue, with or without the staff.
	(= seconds 3)
)



Another interesting difference: After state 4's doVerb(), it disposes the three arrays in the same block.


I realized there is no cue there - in either Trigger or Open. The called doVerb() method is expected to do the handsOn() and replace the room script... otherwise the stalled script would block any further spellcasting.

Example: script 800 - pSquidStone::doVerb()

(82
	(g2_myCurrentRoom setScript: (ScriptID 11) 0 self)  # castTriggerScript
	(return 1)
)
(-82
	(if (== (pSquidStone cel?) 0)
		(pSquidStone setPri: 110)
		(g2_myCurrentRoom setScript: (ScriptID 806 0))  # sToppleSquid
	else
		(g2_myCurrentRoom setScript: 0)

		# "The standing stone has fallen to form a sort of
		#   stone bridge up to the ledge."
		(g91_gloryMessager say: 4 1 41 0)
		(g1_Glory handsOn:)
	)
)

comment:11 by Vhati, 6 years ago

Okay... To make Trigger work like Open, I wrote a patch that inserts a "seconds" assignment and erases the self arg.

The staff disappears because doVerb() leads to a normalize.


script 600 - aGate::doVerb()

(82
	(if local7
		# "Something about being with Katrina makes all thought
		#   of magic swirl around in your head. You don't seem to
		#   be able to cast any of your spells."
		(g91_gloryMessager say: 12 6 28)
	else
		(= g441_myX ((User curEvent?) x?))
		(= g442_myY ((User curEvent?) y?))
		(= loc5_action 9)
		(g2_myCurrentRoom setScript: (ScriptID 11) 0 self)  # castTriggerScript
	)
)
(-82
	(g0_hero trySkill: 22)  # myTriggerSkill
	(g2_myCurrentRoom setScript: sThrowIt)
)



script 600 - sThrowIt::changeState()

(1
	(g0_hero normalize:)
	(= seconds 2)
)
(2
	(if (proc64999_5 loc5_action 1 2)
		# "Your missile vanishes as if by magic.
		#   (In fact, it probably IS by magic.)"
		(g91_gloryMessager say: 12 6 6 0 self)
	else
		# "Your spell dissipates as if blocked by magic.
		#   It does not seem to have had any effect."
		(g91_gloryMessager say: 12 6 7 0 self)
	)
)

comment:12 by Vhati, 6 years ago

Another bug. On successive castings, the green projectile isn't visible.

After the room is reset by hero re-entering, it will be visible again only for the first casting.


This is weird.
triggerEffect ignores the first dispose() call.


script 11 - triggerEffect::dispose()

(method (dispose)
	(if local3 (super dispose:) else (= local3 1))
)

The Open spell has the same bug and dispose() weirdness.

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

comment:13 by Vhati, 6 years ago

I patched them to super::dispose() always, and that made both spells' effects consistently visible.

Another bug.
The cursor lingers after the first casting.
Then it advances after the second casting.
I don't know why they're different.

Somehow it's not getting cached or the cache is erased.


handsOff() calls proc0_7() to cache the cursor and sets global438=1 to ensure that consecutive handsOff() calls don't re-cache.

script 0 - proc0_7()

(procedure (proc0_7)
	(if
		(not
			# Open, Trigger, Fetch are ALWAYS exempted!?
			(proc64999_5 ((User curEvent?) message?) 80 82 87)
		)
		(if (g69_mainIconBar curIcon?)
			(= gl204_myCachedCursor (g69_mainIconBar curIcon?))
		)
	else
		(= gl204_myCachedCursor 0)
	)
)



handsOn() resets global438 and calls proc0_8(), which restores the cursor and clears the cache.

script 0 - proc0_8()

(procedure (proc0_8)
	(if g204_myCachedCursor
		(g69_mainIconBar curIcon: g204_myCachedCursor)
		(g1_Glory setCursor: (global69 getCursor:))
		(= g204_myCachedCursor 0)

		# If the cached icon was for the item button,
		# but it's not showing an active item, advance the cursor.
		(if
			(and
				(==
					(g69_mainIconBar curIcon?)
					(g69_mainIconBar at: 6)
				)
				(not (g69_mainIconBar curInvIcon?))
			)
			(g69_mainIconBar advanceCurIcon:)
		)
	)
)

comment:14 by Vhati, 6 years ago

Here's what's going on with the cursor.

handsOff() was intended to never cache the cursor when the current event message is from Open, Trigger, or Fetch. Sometimes it doesn't recognize them.

  • At every moment, User::doit() recycles "curEvent" and passes that to User::handleEvent().
    • User's "curEvent" property is a vEvt object.
    • vEvt::new() calls kGetEvent to clobber all its own properties with fresh values.
      • Among them are: type, message, modifiers, x, y.
      • When idle, the coords match the cursor's position at each moment, and the rest are 0.
  • The event is passed around until it reaches aGate, which defers to Feature::handleEvent().



script 64950 - Feature::handleEvent()

(method (handleEvent param1 &tmp temp0)
	(cond 
		((param1 claimed?) (return 1))
		(
			(and
				(& (param1 type?) $4000)
				(self onMe: param1)
				(self isNotHidden:)
			)
# Important.
			(CueObj
				state: 0
				cycles: 0
				client: self
				theVerb: (param1 message?)
			)
			(= temp0 (param1 claimed: 1))
			(if
				(and
					(g80_User canControl:)
					(& (g0_hero state?) $0002)
					(>
						(GetDistance
							(g0_hero x?)
							(g0_hero y?)
							approachX
							approachY
						)
						approachDist
					)
					g66_gloryApproachCode
					(& _approachVerbs (global66 doit: (param1 message?)))
				)
				(g0_hero
					setMotion: PolyPath approachX (+ (global0 z?) approachY) CueObj
				)
			else
				(g0_hero setMotion: 0)
# Important.
				(if (self facingMe:) (CueObj changeState: 3))
			)
		)
	)
	(return temp0)
)



  • CueObj is a Script that holds the message (82) until state 3, when it calls aGate::doVerb(82).
  • If hero is already facing aGate, that happens immediately.
    • CueObj is skipped directly to state 3.
    • doVerb() schedules castTriggerScript.
    • handsOff() checks if ((User curEvent?) message?) is 82.
    • The cursor is not cached. This seems to be intended behavior.
  • However, if hero is not facing aGate, there is a delay.
    • CueObj state 1 adjusts hero's heading to face aGate.
    • In that extra time, User::doit() resets the event, zeroing out "message".
    • CueObj reaches state 3. Yadda yadda yadda...
    • handsOff() sees ((User curEvent?) message?) is 0. It WILL cache the cursor.



Bonus confusion: hero::normalize() doesn't reset "heading".

After the first casting, hero appears to face north, but technically still faces aGate.
So the next casting will not cache the cursor.


That's more or less what's going on. aside from one case I don't understand... If hero stands below the gate facing north and casts at a spot directly overhead, all castings cache the cursor.


I'm not confident about messing with handsOff and the event system to fix it.
I'll leave it be.

It can be reported as a separate bug someday if it turns out to be serious or if an easy patch is imminent.

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

comment:15 by Vhati, 6 years ago

Keywords: has-pull-request added

Pull Request: SCI32: Fix QFG4 Trigger and Open spells

comment:16 by bluegr, 6 years ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed

comment:17 by tomasz89, 6 years ago

Agree that the lockup no longer occurs, however, the Hero should retain the staff in hands after casting. This works for Open, but not Trigger.

Note: See TracTickets for help on using tickets.