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
