Categories
Week 5

Fixing Scripts, Text, and Finding Gus

This week had a mix of bug fixes, debugger improvements, and game detection work. Let me walk through what happened.

Before I get into bugs, a quick note: I will be posting a separate blog on the visual debugger soon. Here is a snippet from a data flow diagram for the dt:

An Unresolved Bug

While testing Gus Goes to Cybertown’s Science Dome, I found a bug in the Sink or Float minigame: dropping the golf ball or spoon shows Prof. Gus’ face instead of the drop animation.

I traced it to cast member 525 (s_fqt, a digital video) on channel 14, which spans frames 7350 – 7359 during the drop animation.

In the original Director 4, this sprite is effectively invisible, its bounding box appears as three vertical dots on stage, not selectable or resizable. For the spoon case, no bounding box appears at all. ScummVM renders it at full size (240×180) instead.

The score stores the correct dimensions, setCast is called correctly, and the bbox passed to createWidget is right. So ScummVM is reading the data correctly – the original Director is doing something to suppress this sprite that we aren’t replicating yet. The likely culprits are the ink mode or a puppet flag, but I haven’t confirmed which yet.

Sev’s advice was to study the score at the exact frame where the original renders correctly; check ink, dimensions, and puppet flag,  and look for any Lingo touching that channel. If the cause still isn’t clear, the plan is to strip the movie down to just the affected frames and cast members as a minimal test case, which can then live in the test suite to prevent future regressions.

This one is staying open for now. Sev also pointed out that since this bug is complicated, it’s worth finishing a sweep of all the Gus games first to find something simpler to close out, which is the plan for the coming week.

Text Wrapping 

The next bug I tackled was text wrapping in D4 text cast members. Some labels like “Hamburger” were incorrectly wrapping as “Hamburge\nr”, one character too early.

The root cause turned out to be two separate issues. MacText treats maxWidth as the outer width and subtracts border, gutter, and shadow internally, so I was passing the wrong value and the effective inner wrap width was narrower than the cast member’s designed text area.

On top of that, wordWrapTextImpl was using > instead of >=, meaning text that exactly fills the available width was being wrapped onto a new line when it shouldn’t be.

Both are easy fixes in isolation, but finding them required understanding how the text layout pipeline chains together across three layers. The fix ships with a regression test in director-tests.

Debugger (DT): Script Viewer

While testing Gus Goes to Cybertown, clicking on some cast members in the Cast window showed the script tooltip on hover but opened nothing on click. The script viewer window stayed empty.

The issue has two parts. First, scripts using internal generic event handlers (scummvm_generic) were failing because getHandler() matched by handler.name == handlerId exactly, but generic event handlers store an empty name and are only identified by the isGenericEvent flag.

Matching on that flag when handlerId is “scummvm_generic” fixes it.

Second, some cast members return null from getScriptContext(), the root cause is still under investigation, but the scripts do exist and their source text is available in CastMemberInfo::script.

As a workaround, addToOpenHandlers() now falls back to displaying the raw Lingo source text when no compiled AST is available.

Sev shared a different D8 movie that had the same issue, which confirmed it’s not game-specific. The workaround covers that case too.

Gus Goes to Cybertown: Detection Entries

Gus Goes to Cybertown has three distinct Windows versions: Retail (Director 3), Retail Revised (Director 4), and Golden Master (Director 4). Each has different file sizes and hashes, so three separate detection entries were needed.

That was a summary of most of the things I did this week.

My next immediate goal is to fix the remaining gustown errors, and add it to release.

PRs this week:
Fix text wrapping in D4 text cast members
D4-win director-tests repo changes for the above PR
Fix script not rendering in script viewer
Add detection entries for Gus Goes to Cybertown

Categories
Week 4

Game Detection and ImageDiff Integration

Tuesday

Not a lot happened this week because I was travelling.

the hfs file system

The file system used by old mac computers was the Hierarchical File System (HFS). And for director engine that meant a bunch of issues, like allowing the weird file names (solved by punycode) and resource forks.

Mac files have two parts: a data fork and a resource fork. For Director games, the projector executable lives in the data fork, but the startup movie (the first thing the game loads) is in the resource fork.

When ScummVM detects a Mac Director game, it hashes the resource fork specifically (using the r: prefix in detection entries) rather than the data fork, because the data fork is just the generic Director player and would be identical across many games.

This week I added detection entries for Gus Goes to Cyberopolis, both Windows and Mac versions. The process was interesting: you add a placeholder entry with a garbage hash, compile, point ScummVM at the game folder, and it reports back the real hash and file size for you to fill in.

I also learned why two-file detection entries (MACGAME2, WINGAME2) matter: with a single file, ScummVM just picks whichever game has the most files matching, which can cause false matches when multiple games share a disc. Two files makes the match unambiguous.

For Windows, it’s a bit different. The .exe file is actually three things concatenated together: the generic Director projector code, the startup movie, and a small header at the end that stores the offset to the movie. The executable would read its own tail to find the header, then seek back to load the startup movie. This is why ScummVM can’t hash the first 5000 bytes of a Windows Director executable, they’re always identical across every game built with the same Director version. Instead it hashes the last 5000 bytes (t: prefix), which come from the embedded startup movie and are unique to each game.

ImageDiff Integration (final)

I also worked on integrating ImageDiff into the ScummVM buildbot.

I have mentioned about the imagediff tool in my past blogs in detail. It was currently running on a detached tmux session, which is kind of a hack, so sev gave me some resources to read and integrate the tool into actual buildbot.

This process unfortuntely took a lot of time for me, because this was my first time in a long time dealing with a different kind of code.

The integration used a buildbot plugin called buildbot-wsgi-dashboards, which lets you embed a Flask web app directly into the buildbot UI. Getting it to work involved fixing a few issues. Most of the issues were trivial but there was this one issue which was causing a lot of problem and I wasnt able to figure out the root cause for, for the longest time.

The issue was, whenever I loaded the imagediff page on the buildbot, it would load unreliably i.e every time I would refresh the page, I couldn’t predict whether I would get a “Resource not found” error or the page would actually load. On top of that the table wasn’t loading properly.

The fix was actually two separate issues. First, the environment variables weren’t being loaded early enough, the SCREENSHOTS_DIR variable was being read at import time before the .env file had been loaded, so it defaulted to ./screenshots/ which didn’t exist on the server. Adding load_dotenv() at the top of config.py fixed that.

The second issue was the JavaScript on the frontend constructing API URLs without the correct path prefix. Since ImageDiff is served under /plugins/wsgi_dashboards/imagediff/, a hardcoded /api/target_data/... URL would 404 every time. The fix was deriving the base path dynamically from window.location.pathname at runtime. That’s why the page was loading unreliably, depending on timing and caching, sometimes the old URL worked by accident and sometimes it didn’t.

After sorting all of that out, ImageDiff is now properly integrated into the ScummVM buildbot at john.scummvm.org. But it’s still rough around the edges, and the remaining issues will be handled by sev.

PRs:

Imagediff

Detection Entry

 

Categories
Week 3

Read the Error

Wednesday

This is the follow-up to the previous post.

The Blunder

The first task was integrating ImageDiff into the buildbot repo.

The first PR was wrong immediately. I added the files directly without bringing in the git history from the original ImageDiff repo. Sev had asked for history. The PR wasn’t mergeable, and sev fixed the git situation himself rather than have me fight with it. First mistake.

Then the actual blunder. When wiring ImageDiff into the buildbot’s Python pipeline, I hit an import error imagediff.py does from config import SCREENSHOTS_DIR at module level, which breaks when imported from a different directory. Instead of reading the error and fixing it, I asked an AI, got an importlib workaround, and pushed it.

Sev said in the chat, “please don’t do that anymore, asking AI and pushing the slop I mean.”

The correct fix was a one-line sys.path insert, something I’d have found in two minutes if I’d just read what the error was saying.

Deployment

After the ImageDiff PR merged, I was hesitant to deploy directly on the buildbot server as I didn’t want to break anything.

In response I was given the green light to break it, because we already have a VM snapshot saved.

Deployed, it worked, buildbot was up with ImageDiff integrated.

The Misunderstanding That Cost the Most Time

Once it was running, sev noticed the tool was timing out on target: theapartment-mac, he found it and pointed toward the cache logic.

That was a part of it. The cache logic was kind of flawed. But we had a bigger elephant in the room.

The real problem was something I hadn’t understood about how the tool was supposed to work at all.

The movie_diff() function was opening every PNG frame from both builds with PIL, running ImageChops.difference() on each pair, and checking to detect any pixel difference.

But it was completely wrong. The Director engine already does the pixel comparison during playback, in score.cpp. It compares each new frame against the stored reference using a pixel difference threshold, and only saves the file to disk if the difference exceeds that threshold.

So, file presence is the diff signal.

The fix was replacing all the PIL logic with: does any file matching {movie}-*.png exist in the comparison build’s directory?

I didn’t know the engine had this mechanism. PIL was redundant work the engine had already done.

The Punycode Crash

After the performance fix, a new crash appeared:

ValueError: chr() arg not in range(0x110000)

The movie name causing it was xn--xn--File IO-oa82b-. Sev explained why this exists.

Mac HFS allowed file names with characters that would be illegal on any normal filesystem  /, *, newlines, etc. “The Apartment” has movies named things like File I/O and •Main Menu.

To store these on a normal filesystem, sev designed an encoding scheme based on Punycode: files with special characters get an xn-- prefix, the special characters are removed from the name, and their positions are encoded in the tail.

xn--xn--File IO-oa82b- is doubly encoded, it went through the encoder twice, which is valid, it just needs two decode passes to get back to File I/O.

The bug was in decode_string(). There was a loop that stripped trailing dashes from the string before handing it to the punycode decoder:

i = len(orig) - 1
while i >= 0 and orig[i] == "-":
i -= 1
orig = orig[:i+1]

For xn--xn--File IO-oa82b-, this strips the trailing - and corrupts the input before the decoder even runs. Removing it was the entire fix. Without the loop, Python’s punycode decoder handles the string correctly.

Director Debugger

Three things on the dt-new branch:

Cast Details Panel: was showing ... for almost everything. Now shows : script text previews on hover with click-through. The old showScriptCasts pipeline was removed entirely everything now goes through renderScript.

Scripts Window: decoupled from the execution context, which previously shared state with it. The scripts window now has its own handler list and browser-style back/forward navigation.

Also fixed a bug where the Lingo/Bytecode toggle was a single global flag shared across all open handlers.

Keyboard Shortcuts: Cmd+2/3/4 toggle Control Panel, Cast, Score. On Mac, cmd instead of ctrl must be used.

PRs

scummvm-sites #39 · #40 · #41 · scummvm #7577

 

Categories
Week 3

Before I disappear

Tuesday

No, this is not my last post. I am just rushing because today is the last day to post for the previous week and I am not sure how long will I have a stable internet connection.

I will not be online (probably) for the next 2 days, because I am in a very remote location (mountains) so internet access might be a problem. So, this post will get straight to the point.

I made a very big blunder this week, and a lot of code was rushed. But the week ending was good. Got to learn a lot. And very interesting story to tell.

I will make a detailed post for this week after I get a stable internet connection.

See you soon.

Categories
Week 2

Following the Warning Lines

Tuesday

This week I finally got SSH access to john.scummvm.net, the machine that runs the Director buildbot. I’d seen its output for weeks (those BUILDBOT: warning lines I kept triggering) but had no idea how it actually worked. Getting access and reading through the code was very satisfying because I finally met the machine that had been yelling at me for the past couple months.

The Buildbot Architecture

buildbot_diagram

How a build starts

The first thing that clicked was how the build is scoped. The buildbot watches the ScummVM GitHub repo via a webhook, but it doesn’t rebuild on every commit. master.py checks whether any changed files are in engines/director or graphics/macgui. If yes, it kicks off a build. If not, it ignores the commit entirely. Simple filter, makes sense.

Triggering the tests
Once the ScummVM binary is compiled and uploaded to the master, build_factory.py calls steps.Trigger(schedulerNames=["Director Tests"]). This tells the buildbot master to fire the Director Tests scheduler, which is a Triggerable scheduler defined in master.py. That scheduler then queues up all the test builders at once; one for each entry in targets.json, so they all start running in parallel on the available workers. The build step waits (waitForFinish=True) for all of them to complete before marking the overall build as done.

Running the tests and error checking

The test runners themselves are defined in targets.py. Each one downloads the binary, rsyncs game files from /storage, and runs ScummVM against a list of movies. Screenshots are saved to /home/director-buildbot/screenshots/ on every run. Error checking is done in steps.py, which watches the output for lines matching “BUILDBOT: incorrect check for line:” – that’s the log-replay mechanism from the director-tests repo, where expected output is recorded directly into the test movie file, and on each buildbot run the live output is compared against it line by line.

Reading through this, the separation of concerns became clear: master.py handles scheduling, build_factory.py handles the build pipeline, targets.py defines what gets tested, and steps.py defines how a single test run behaves. Once I had that mental map, the rest followed quickly.

The ImageDiff tool

pic of the tool from dev chat

One thing the buildbot produces but doesn’t analyze automatically is screenshots. Sev pointed me to ImageDiff, a tool built to catch visual regressions, cases where the engine produces slightly different output without triggering any log-based errors.

It’s a Flask web app that reads from the screenshots directory and shows a frame-by-frame diff between any two builds for a given target and movie. The core logic uses PIL’s ImageChops.difference to compute a pixel-level diff. If there’s any difference, the diff image is rendered alongside the source and comparison frames so you can see exactly what changed. Results are cached to disk so repeated comparisons don’t recompute.

I temporarily deployed it on john.scummvm.net on port 5002. The two targets currently generating screenshots are director (from the director-tests-* folders) and theapartment-mac, both of which showed up with their full build history. It’s a simple tool but it fills a gap that log-replay can’t, visual changes.

Bugs

This week I did not work on any game bug fixes. I only made changes to the visual debugger and its bugs.

The gus games bugs are mostly fixed already and the remaining one bug has not been very consistently reproducible. So, this week I’ll try to find out how to replicate it.

Here is a brief on the DT changes:

Windows Panel

  • Added a new Windows panel showing all currently loaded windows and all .DIR movie files in the game directory
  • Clicking a movie navigates to it via func_goto

Search

  • Added variable search mode to the search bar
  • Improved search with new modes (Handlers, Variables, Body) and a cleaner 3-column results table with keyboard focus on open

PRs:

https://github.com/scummvm/scummvm/pull/7564

https://github.com/scummvm/scummvm/pull/7553

last week’s changes: https://github.com/scummvm/scummvm/pull/7563

What I am currently working on

  • Currently I am working on some more DT changes.
  • Working on the checksum function.
    • Some Director movies have a VWCF resource with a checksum that our implementation fails to verify correctly, causing a crash when navigating to those movies in the debugger.
    • Sev suggested – before diving deep – running the mismatched movies through ProjectorRays first to see if it also miscalculates, that way we know whether the bug is in our implementation or the movies themselves. The code once completely written, will be identical to the Projector Rays checksum code.

P.S. The buildbot integration will be re-worked soon by one of our devs rvanlaar, so the current architecture might not be relevant in the future