Opened 5 years ago

Closed 5 years ago

#10778 closed defect (fixed)

QFG4 floppy: Crashes on the first fortune teller visit

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

Description

ScummVM 2.1.0git3797-ge7d23d2cd9 (Oct 25 2018 04:17:12)
Windows 7 64bit
QFG4 Floppy 1.1a + note patch (English)

During the first visit to the fortune teller...

When you click MOUTH on the hero to "Say goodbye", Davy will insist that you stay in the camp for a dance, and there's a crash.

"ERROR: kAddScreenItem: Plane 0000:0000 not found for screen item 0013:1aa9!"

If you WALK away, the fortuneteller says farewell. Same error.

If you try to give money to the fortune teller (for a reading), it crashes immediately. Same error, different cause?

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

Attachments (2)

sci.071 (51.2 KB ) - added by Vhati 5 years ago.
SavedGame (Floppy) - Fortune Teller
sci.087 (54.9 KB ) - added by Vhati 5 years ago.
SavedGame (Floppy) - South of the camp

Download all attachments as: .zip

Change History (15)

by Vhati, 5 years ago

Attachment: sci.071 added

SavedGame (Floppy) - Fortune Teller

comment:1 by Vhati, 5 years ago

The CD edition under ScummVM did not have this bug.


ScummVM 2.1.0git3797-ge7d23d2cd9 (Oct 25 2018 04:17:12)
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

comment:2 by bluegr, 5 years ago

Summary: QFG4: Crashes on the first fortune teller visitQFG4 floppy: Crashes on the first fortune teller visit

comment:3 by Vhati, 5 years ago

Using this as an exercise in learning the debugger and SCI Companion. From what I can gather...

The hero object (0013:1aa9) is hidden in the wagon room's init() (room 470). When the room is disposed, it calls hero::show(), but at that point, hero has no valid plane to be drawn on.

I'll attach a savegame south of the camp so entry can be debugged.

by Vhati, 5 years ago

Attachment: sci.087 added

SavedGame (Floppy) - South of the camp

comment:4 by digitall, 5 years ago

Vhati: Glad you are learning the debugger and taking a look at SCI scripts. You may want to refer to the following along with the ScummVM SCI engine source code:
https://wiki.scummvm.org/index.php?title=SCI/Specifications
https://wiki.scummvm.org/index.php?title=SCI/FreeSCI

Patches as per https://github.com/scummvm/scummvm/pull/1353/commits/8c49763a882e92cc09b3468218895f9e0691152d would be gratefully received... but if this is too hard, thanks anyway for the level of detail on your bugs since it will make tracking this down much easier for SCI engine developers.

comment:5 by Vhati, 5 years ago

OFF-TOPIC

@digitall:
I'm not at a place where the specs can help me yet.

I did find this article particularly helpful, demonstrating backtrace (bt) and breakpoints (bp_method) in SCI (Island of Dr. Brain). Wish there were more like that!
https://moral.net.au/writing/2017/09/23/sierra_bug/

Plus the subtle hint that "view_object 0013:1aa9" can be written as "view_object hero". And further, that one can drill down into attributes with "view_object hero plane", etc.


The send command sounds like it could open up cheaty fun, invoking stuff on the fly, if I had a clue how to wield it... *clickety-click*

visible_plane_list
# Hm.
visible_plane_items statusPlane
# Hmm.

send statMana hide
go
# Looks at the absent mana bar.
send statMana show
go
# Heh heh.
# Experiments with listing inventory icons that way and calling their loseItem.
# No get(). Those NumInvItem View objects must be downstream of the real
# inventory management.
# Hrm. "send hero get theOil" crashes.
# Peeks at the cave arch script for properly getting the Dark One Sign item.
view_object hero
send hero drop 25
go
# Checks inventory.
send hero get 25
go
# It's back.
# 1, 2, 3...
send hero get 15
# Struck oil!

send hero has 15
# Message completed. Value returned 0000:0002.
send hero get 15
send hero has 15
# Message completed. Value returned 0000:0003.

*evil grin*


Aw. Looking at script 28 where the hero class is defined... takeDamage method might be using negative args to heal. Debugger can't pass 'em because its parser chokes on the minus sign.

Maybe I'll post questions on the forum sometime: pagination/redirection for long text, setting attribute values on objects, passing negative args, etc.

comment:6 by Vhati, 5 years ago

@digitall:
I'm a long way off from submitting patches myself. Admittedly some of my trivial tickets might be good "Hello World" material.

For now, while I wait for playthrough-blocking bugs to get fixed, I can become a power user to hack around them, report in greater detail, or at least entertain myself trying.

comment:7 by m-kiewitz, 5 years ago

Can you please try to warp to that location using debugger and check if the bug happens all the time?

We need to check if maybe Sierra simply ignored such calls in the original interpreter.
I wouldn't be surprised if they did, because they almost never checked for anything at all.

comment:8 by Vhati, 5 years ago

@m-kiewitz:

try to warp to that location

Quick and dirty way to skip to the first meeting with the fortune teller.

  • Create a new character. Any kind.
  • Wake up in the starting room.
  • Set plot flag 39 (freeing Davy from the Burgomeister's cell)
    • vv g 502 256
  • Get rich.
    • send hero get 0 100
  • Teleport to the forest south of the camp.
    • room 588
  • Dismiss the debugger, and save.
  • Walk north.
  • Davy will welcome hero in. Both will enter the fortune teller's wagon.
  • The above steps will cause rm470::init() to set local0 = 1 (first meeting).
    • Walking out will crash.
    • Paying the fortune teller for a reading will crash.
    • Teleporting away will crash.
    • Restarting will crash.

All crashes involve hero having a null "plane" property, when hero::show() is called, from rm470::dispose().

The CD edition does not crash.


script 470 - rm470::dispose()

(method (dispose)
	(g0_hero posn: 1000 1000 show:)
	(g93_walkHandler delete: self)
	(super dispose:)
)

Diffing against the CD edition... Only the floppy has that posn & show line.

I patched that line away. No more errors.

comment:9 by Vhati, 5 years ago

@m-kiewitz:

We need to check if maybe Sierra simply ignored such calls in the original interpreter.
I wouldn't be surprised if they did, because they almost never checked for anything at all.

The floppy edition's original interpreter did not crash.

comment:10 by Vhati, 5 years ago

Keywords: has-pull-request added

Pull Request: SCI32: Fix QFG4 fortune teller crash

comment:11 by m-kiewitz, 5 years ago

@Vhati
We need to look into the actual code of the original interpreter.

This here may be caused by:

  • something else going wrong and thus Plane 0:0 gets used for that screen item and should not
  • Sierra ignored it when Plane 0:0 got used, and we shouldn't error() out.

As I said, we check tons of things that Sierra did not check. Certain script bugs were not showing up in the original interpreter because of this.

in reply to:  11 comment:12 by Vhati, 5 years ago

@m-kiewitz:

This here may be caused by:

  • something else going wrong and thus Plane 0:0 gets used for that screen item and should not
  • Sierra ignored it when Plane 0:0 got used, and we shouldn't error() out.

In that case, #10773 would be relevant.

comment:13 by bluegr, 5 years ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed

Thanks for your work! The pull request has been merged, so this bug can be closed now

Note: See TracTickets for help on using tickets.