#14020 closed defect (fixed)

SCI: Jones in the Fast Lane store item animations cycle far too quickly

Reported by: Purduecoz Owned by: sluicebox
Priority: normal Component: Engine: SCI
Version: Keywords:
Cc: Purduecoz Game: Jones in the Fast Lane

Description

The pictures of the items that cycle through when you stop at a store go so quickly that you can't really even see the items. Guessing this has to do with some of the SCI timer change commits that were made recently.

Change History (10)

comment:1 by sluicebox, 16 months ago

Thank you for reporting this. This is an unthrottled inner loop, so it also goes really fast in the original on fast computers. This only affects the CD version.

You're right, recent changes in August have exposed this. No more hiding behind kGetEvent throttling meant for fast-cast mode!

---

Update: Yikes, they removed the kWait(1) call from the Dialog class' event loop in the CD version. That's what keeps SCI dialogs from doing this. Maybe they tried to make it go a little faster on their 386's by removing all throttling? Fun!

I'll scan other games to see how widespread this is. I know that much later games have this missing line of code, so this could just be a Jones CD hack. Either way, patching a pause back into Dialog:doit would fix this.

Last edited 16 months ago by sluicebox (previous) (diff)

comment:2 by digitall, 16 months ago

I reported this on IRC last night as I spotted this regression as well.

Apart from the speed issues, I think this is meant to show the item picture which you have the mouse hovering over, rather than just cycling through all the items in the current store square.

This seems to work albeit very slowly to respond with older versions.

Trying to bisect this now.

Apart from this, there were two warnings of:
WARNING: Uninitialized read for parameter 6 from method select3::open (room 1,
script 255, localCall ffffffff)!
WARNING: Uninitialized read for parameter 6 from method fastFood::open (room 1,

script 255, localCall ffffffff)!

Though these were fixed(?) by commit 30fad94e9a768d67287906c15386ae212af8fbac
Author: sluicebox <22204938+sluicebox@…>
Date: Sun Nov 27 19:16:55 2022 -0800

SCI: Track correct location and size of temp variables

Though not sure if that is related.

Replication steps I am using. I can replicate with v2.5.0 and v2.4.0git:

  1. Start Game.
  2. Click on "Jones in the Fast Lane" logo
  3. Click on "Play Game"
  4. Click on "1"
  5. Click on Left Character "Select"
  6. Click on "Play" i.e. standard Goal settings
  7. Click "No" on Play Against Jones
  8. Click on Monolith Burger (right side)
  9. Allow the MB Clerk speech
  10. At this point, the item shown is fixed at the first burger
  11. Hover over an item.
  12. The item show changes picture to that but then starts cycling through each item

in turn slowly... which I don't think is correct.

Last edited 16 months ago by digitall (previous) (diff)

comment:3 by digitall, 16 months ago

Ah, this video looks to be running in original interpreter under Win3.1:
https://www.youtube.com/watch?v=CaGQYgcbTpU

It looks like the original behaviour is to cycle the items and only show a specific item once clicked on to buy?

@sluicebox: Do you concur from looking at the scripts?

comment:4 by sluicebox, 16 months ago

Owner: set to sluicebox
Resolution: pending
Status: newclosed

I have a fix for this and similar inner loops, I'm going to test it for a few days before committing. I had considered doing this earlier, but it wasn't clear if it was necessary. Now it's clear! One of the themes of the recent timing changes is to limit our countermeasures to just their intended targets.

@digitall: This game, and others, generate a lot of uninitialized parameter reads warnings. It's not related to this or recent commits. That's just what it does in the original. Most of the time they read an out of bounds value and then don't use it, so it doesn't matter.

I'm under the impression the graphics are just supposed to cycle after delays, and change when you click an item. In the original under DOSBox, the graphics don't change when I mouse over selections.

It looks to me like Sierra tried to reduce the delay in between items on this screen in the CD version. They definitely changed the speed values on this screen. If that wasn't fast enough for them, I can see them removing the kWait(1) line. I scanned all the games for comparison; this is the only one where they just removed it. Unfortunately, that means that it kept getting faster with faster CPUs. After my fix, which will restore the earlier speed, maybe we could do a small script patch to speed up this animation as intended. But I'll leave that up to you Jones experts. =)

While I have you here: Is Jones CD audio working well for you? On the market screen, all the speech is stopping one or two seconds earlier than it should. I get the same results with really old ScummVM versions. Is that just me? (Maybe I don't have the CD mounted right)

comment:5 by sluicebox, 16 months ago

Resolution: pending
Status: closednew

comment:6 by digitall, 16 months ago

@sluicebox: Ah OK. I concur with your comments regarding original interpreter behaviour i.e. it should cycle the items (but much slower) and only show the item if clicked on to buy.

As for Jones CD audio, it is working fine here. I tested in both Z-Mart and Black's Market and the audio is playing fine.

comment:7 by digitall, 16 months ago

@sluicebox: I have checked my original CD data against my game data and found some notes which might explain.

I dumped this from the CD as a bin/cue image.
The ISO and CDR tracks can be extracted from these using the following command:
bchunk -s <bin name> <cue name> <basename for extracted iso/cdr>

However, this version of Jones CD has a mastering error which results in garbage at beginning and end of CD track.
This can be trimmed with audacity, but is not totally necessary
as the data contains a (lower quality) speech audio.

See bug #3069435 "JONESCD: Spoken dialog starts at wrong point"

I have generated a file, track1.mp3 which is 37m48s long for the audio data which I think is being used.

comment:8 by digitall, 16 months ago

@sluicebox: In the newer TRAC bugtracker, this is bug #5367 which has useful information:
https://bugs.scummvm.org/ticket/5367

comment:9 by sluicebox, 16 months ago

@digitall Thank you! #5367 describes exactly what I'm hearing. Glad it's not something new. Although, mastering error or not, it sounds fine in DOSBox. Maybe there's room for improvement? I'll add it to the list. =)

comment:10 by sluicebox, 16 months ago

Resolution: fixed
Status: newclosed

Fixed in: https://github.com/scummvm/scummvm/commit/6eea9e517bdbad20b06170f2f2d3ba5d01eb1543

Testing went well, and there's talk of an upcoming 2.7.0 release, so I don't want to postpone getting this in the development builds.

Thanks again for reporting this!

Note: See TracTickets for help on using tickets.