[Scummvm-tracker] [ScummVM] #10814: QFG4: Crash in cave when fighting Pit Horror below the tightrope

Vhati trac at scummvm.org
Fri Nov 23 02:58:11 CET 2018


#10814: QFG4: Crash in cave when fighting Pit Horror below the tightrope
--------------------------------+-------------------------
  Reporter:  Vhati              |      Owner:  (none)
      Type:  defect             |     Status:  new
  Priority:  normal             |  Component:  Engine: SCI
Resolution:                     |   Keywords:  SCI32
      Game:  Quest for Glory 4  |
--------------------------------+-------------------------

Comment (by Vhati):

 @m-kiewitz:
 > It's always active, so why even bother to check.

 I'm warming to the hypothesis that it's not really a condition to 'check'.
 The OR hs a micro-optimizing hack to sneak a void func into the middle of
 a list of conditions for an if block.
 \\
 {{{
 if (register && (SetNowSeen(horror) | 1) && (boundary tests)) {
         register = 0          // Got a hit, stop checking.
         g188_egoSpell::cue()  // Advance the spell Script's state to
 disappear?
 }
 }}}
 \\
 * If "register" is 0, short-circuit.
   * doit() runs frequently, I imagine. Every optimization helps on slow
 hardware.
   * Script objects in script 855 tend to set their own "register" property
 occasionally when changeState() wants to toggle the frequent collision
 detection.

 * The func is there to update boundary properties so that collision tests
 always have current data, but only when "register" deems it necessary.

 * The author may have been acting out of a general sense that the truth
 evaluation of void funcs is erratic.
   * In the specific case of what I disassembled above, SetNowSeen() could
 exist as a pseudo-condition unaided, always non-zero. This is only because
 its stack was pushed immediately before the callk, with an object arg.
   * Wrapping a void func in a bitwise OR ensures that however it compiles,
 it will evaluate to non-zero.
   * If things don't explode when you do SetNowSeen(null), an OR wrapper
 would stop that zero from leaking to wreck the if block.
 \\
 \\
 Why not this? Dunno. Brevity?
 {{{
 if (register) {
         SetNowSeen(horror)

         if (boundary tests) {
                 register = 0
                 g188_egoSpell::cue()
         }
 }
 }}}

--
Ticket URL: <https://bugs.scummvm.org/ticket/10814#comment:19>
ScummVM <https://bugs.scummvm.org>
ScummVM


More information about the Scummvm-tracker mailing list