I’ve changed a lot of files, and because this is a major change expected to break the game somewhat, I’m doing it in a new repository. I’ve been working on my laptop, so the next step is … download the new repository on my desktop machine and make sure everything compiles and runs and can upload a build to Steam.

Except wait, we really should install Visual Studio 2015 Update 3 on this computer first. And we know that takes a long time…

While that is happening, let’s dig out the Braid backup files. I am hoping that I have .bmp versions of all those .jpg files that I just converted back to .bmp (because the conversion to .jpg will have lost data). If I don’t, well, we can always go back to the .psd files, but there are some perils involved there that I will explain when we come to it. (These perils are one of the things that has been buzzing around at the back of my mind for years now, so it will be nice to have them solved).

The Braid files have been backed up mostly on some random hard drive:

When it comes to the source code and shipping assets, all that is on a server in a hosting facility and gets backed up regularly, so it’s very unlikely to be lost. But when it comes to the higher-resolution source assets, older historical files, and whatever, they only exist here (with the source images also on some DVDs that David Hellman burned that are now sitting in a storage crate in a warehouse).

Recently, in the office, to back up really big files for The Witness, I set up a Synology backup server (recommended by Jeff Roberts and I am finding it to work quite well). We’ve got a bunch of free disk space:

(It’s a bit more than we need right now, but the hope is that this will last for some years).

I have made folders to back up all our projects:

So: I am now copying all the Braid backups into there (excluding the backups of the old-style Steam depots, because it used to be a really bad thing if you lost those, but since then Steam has moved to a much less-janky system and these aren’t necessary any more).

It’s been long enough that I am not really sure what is here any more. I am hoping the source art is really here, and sure enough, there’s a folder “Source Art”, let’s see what’s in it…

Hmm, what’s this, “Old Programmer Art”?

Oh, this ought to be fun …

At first glance I am not finding .bmp files corresponding to the .jpgs; I am hoping they are there somewhere and will do a search when the copy from the slow hard drive is done.

In the meantime, Visual Studio 2015 Update 3 is done and it of course wants to reboot my computer. Sigh. Reboot. Double-click on the braid.sln. This launches Visual Studio 2013, which is the wrong version, because Microsoft seems to have broken version selection and never bothered to fix it. Close Visual Studio 2013, open 2015, manually drag the project file onto it. Clean solution. Build.

To nobody’s surprise, the build fails. stb_image.h and stb_image_write.h are missing. Add them on laptop, check in, update on desktop. This time it builds. Set launch options in Visual Studio because those don’t ever get saved anywhere reasonable. The game runs!

This is in unpackaged mode. Let’s run the package-building thing. Oh, it uses File::Slurp and that is not installed by default with Perl.

ppm install File-Slurp

and wait a bit. Then run again. It builds the package in under 10 seconds (this used to take many minutes back in the day!) Go into Package, run braid.exe. Launcher comes up. Press Launch button. Game comes up!

cd ContentBuilder
run_build

Steam updates for a few seconds. Then I get Steam Guarded. Go to email, type the code.

ERROR! App build file does not exist: ..\scripts\appbuild_499180.vdf

So apparently I forgot to check that in as well. Actually it’s about 7 build config files that are missing (for various platforms, etc). Check in, update.

run_build

It’s getting further now. Scanning content. Uploading content. Then Steam says:

ERROR! Failed to commit build for appid 499180 : Failure

I have no idea why it’s failing; also though, I know it really succeeded, because it does this every time. Maybe at some point I will bug Valve about this but ehh. Log into Steam partner site, to check the depot, it takes me a few minutes to find the right page even though I do this all the time, because this site is some in-progress mixture of old UI and new UI and it is remarkably hard to get to the page that actually has the technical stuff on it. The upload succeeded but it wasn’t set live, so I do that. (This is not live to players, just to me, since this is a separate copy of Braid just for experimentation).

Run Steam, download the modified Braid through Steam. When it’s done downloading, it still isn’t ready to run – it wants to download another 62MB. I guess I downloaded it before the change propagated through the magical internet servers. Second download finished. Run it. Launcher box appears. Press Launch. The game runs.

Yay! That is always a bit of a hassle, but we caught a couple of problems … always test things end-to-end on a second machine is a thing that programmers learn pretty early on in development.

Okay, back to the code.

My next step will be to go back over all the loop changes I did yesterday (when inserting that Each macro) and see if I did anything obviously bad. One example of something that could have happened is deleting an extra line of code that really needs to stay in the loop body. Recall that I was editing a bunch of stuff that looks like this:

List *l = get_portables(&ptype_Puzzle_Piece);

Entity_Puzzle_Piece *e;
Foreach(l, e) {
    if (e->portable_flags & PORTABLE_KILLED) continue;
    if (e->piece_index == piece->piece_index) return e;
} Endeach;

and deleting the references to PORTABLE_KILLED. When cranking through hundreds of examples in a row like that, it feels a little too easy to accidentally also delete some other loop condition. Also, sometimes I needed to rearrange lines before the head of the loop, because other stuff would be inserted between the “List *l” part and the Foreach part. This review will be slightly tedious but probably not nearly so much as performing the actual edits. So here goes…

It’s going pretty quickly, because a visual diff program (such as the one provided with TortoiseSVN) will highlight the lines that changed, so I just need to look at the highlit lines to see if there’s anything fishy.

Aaaand … I caught one!

(I accidentally deleted that if (!marker->label) line!) This does not actually cause a crash, because there are no Markers on this level where label == NULL. But it should be fixed anyway. So I am already glad I am doing this check.

(In all projects after Braid, since I have found that testing string labels is something I want to do often in gameplay code, I use a function strings_match() that handles NULL and doesn’t have the weird reverse logic of strcmp’s != 0, which greatly simplifies the situation. In the new programming language, you have empty strings instead of NULL pointers, and == works on strings, so this worry does not come up.)

That’s the only bug I found via this check. Which doesn’t guarantee I didn’t introduce other bugs, but hey. The check only took about 25 minutes, so I think it was a good idea.

Now that that’s done, I want to revisit this idea from yesterday about safely destroying entities, and why I got so confused about this. Let’s say there is a boss monster, and when he plays certain sounds, you want to stop other sounds of his. Well, you’d like to write this:

void eliminate_prior_voice_sounds() {
    Each(issued, Issued_Sound) {
        if (!strings_match(issued->label, "gun_boss_voice")) continue;
        globals.entity_manager->schedule_for_destruction(issued);
    }
}

but if it’s not safe to destroy things while iterating, then I’d have to write this:

void eliminate_prior_voice_sounds() {
    Array <Entity_Issued_Sound *> to_destroy;

    Each(issued, Issued_Sound) {
        if (!strings_match(issued->label, "gun_boss_voice")) continue;
        to_destroy.add(issued);
    }

    Entity_Issued_Sound *issued;
    Array_Foreach(&to_destroy, issued) {
        globals.entity_manager->schedule_for_destruction(issued);
    } Endeach;
}

(complete with Array_Foreach macro that is more verbose than it should be, since in C++11 we can infer the type of the iterated value, and I will clean this up later probably, but it’s not so important now).

The second, longer idiom is being used a lot in the Braid source code; not having worked with it for years, when I saw that idiom repeatedly, I presumed it’s not safe to destroy things while iterating. Well, it is safe, it’s just that after making it safe, I never went back and cleaned up old code. (Braid took 3.5 years to make, and a lot of basic things changed over that time!) The old idiom works, but the problem is, it’s confusing; if it confused me, it is much more likely to confuse people in the future who didn’t write the code in the first place. So it’s time to take care of that.

I imagine the code will shrink a bit when I do this, so let’s get an updated count beforehand: 69,228 lines.

Cool, now I will just hope that I named the array in most instances of this pattern “to_destroy”, and search for that, and simplify. That didn’t take long, so I also do a scan of calls to schedule_for_destruction and catch one that I missed. Game still compiles and runs. Line count: 69,180. (Savings: 48 lines plus potential confusion!)

Okay. I believe it is time to confront the aforementioned peril.

Yesterday I spent a bunch of time banging on the ‘pieced_image_maker’ tool, but I don’t think I ever said what it does. Basically, the job of that tool is to take a .psd file full of stuff that David drew that represented a bunch of different images we wanted to lay out in a level, and pack those images into texture maps in some kind of not-totally-inefficient way, and also output a listing telling us the name of each layer and a set of rectangles along with the areas of the destination texture that those rectangles represent. Any layer can be broken into multiple rectangles, because we don’t want to store or render too many transparent pixels; just dumping each layer as an individual texture would be very inefficient!

For an example, let’s look at some pieces from this scene:

Here’s one of the buildings in the background; it consists of four rectangles.

The rectangle in the upper-right is probably just holding some mostly-transparent pixels that David didn’t mean to be there, but the tool can’t exactly tell what’s a mistake versus what is just subtle, so whereas I put some heuristics in there to trim away things that seem like they are very very transparent, those heuristics are pretty conservative.

Here’s another piece:

Here, you can see that because the rectangles are always axis-aligned, we have a bunch of empty space near the diagonals.

Here’s the ramp in front of that, where we fared a little better:

The tool also splits things when they are too big to fit into the maximum texture size. For example, I am pretty sure this bridge was only split because it was too wide:

In the source art folder that I dug out of the backups, I have all these psd files. So long as they haven’t been thoroughly corrupted, and so long as I haven’t put too many bugs into pieced_image_maker, I ought to in theory just be able to re-run it on all the .psd files, and it’ll generate all the output .bmp files, and that will be great. I can compress those .bmp files to .dds directly, without coming from the lossy .jpgs. And I’ll keep those .bmp files around for future texture compression and storage adventures.

However, the peril is this … the Pieced_Image stores the coordinates of its lower-left corner in the .psd image, so that if we have two Pieced_Images we can reason about where they are with respect to each other (the boss monster in the game, for example, consists of many different pieces, and we render them just by making sure the offsets relative to each other are the same as they were in the .psd). However, the coordinates with which a Pieced_Image entity are stored in the game world does not care about this. It’s super crappy. I am pretty sure (I think I am remembering this correctly) that we just render the image by placing its lower-left corner at the coordinates of the entity, which sounds fine if you don’t think about the fact that the lower-left corner is generated by a program and thus somewhat arbitrary. If you were to paint on a psd layer so that you expanded the bounding box of the resulting image downward or two the left, the images in the game world will move by the corresponding amount.

This is completely terrible, and I am not sure how I ever let this slide, except that David never complained about it much and I always had a lot of other things to worry about.

When it comes to preserving the game into the future this is a massive timebomb. For example, maybe when I run pieced_image_maker it will make exactly the same decisions about how to cut up and pack the images as it did before. Or maybe it won’t. Maybe whatever in Photoshop turned 0xcdcdcd into 0xcecdce will bump the transparency of some pixel below the threshold that my cropping heuristic checks for, and my cropping will change. Maybe I had an uninitialized variable and some arbitrary decisions will come out differently this time.

Now, if that happens, I can tell, but there isn’t currently a mechanism to do anything about it. Suppose I use the lower-left corner of the image as an anchor when rendering … if David then goes and shifts the layer inside the psd a little bit, because he needs more room on the left to draw, then we have the same problem.

In 3D modeling we never have this problem because there’s a built-in idea of an origin and modelers just know what that means. I do know that you can set up markers in Photoshop to mark specific points, so we just need to have one that says “here is the origin in this image” (maybe it starts in the middle) and then images will only ever shift if you move them relative to that origin. So if you need more room to draw, you could make the canvas bigger, and because we aren’t using the corners as meaningful reference any more, this has no effect on the output. As for why we didn’t do this before … bleah, you know the story.

So, here’s what I need to do:

  • At least make sure pieced_image_maker runs well enough to generate some meaningful output.
  • Compare this output to the old output to determine whether anything has moved.
  • If nothing has moved, we don’t have to worry about the next step until later, and can do image compression experiments; otherwise,
  • Build in a system for placing the origin, and then write some code to move every Entity_Pieced_Image in the game by the corresponding offset, so that the Entity’s position refers to where the origin is in the game. I say move all the in-game entities, because otherwise I am just complicating things by adding more weird offsets to each other, which will be harder to understand later. I would rather just have things be simple.
  • Animated characters, and entities like gates and keys, are probably a more ‘entertaining’ situation than this but let’s just start with the above and make sure the levels are okay.

Oh, and while looking over all this, I realized there is some old code in pieced_image_maker.cpp for saving the separate rgb + alpha jpeg images! I’ve just gotten rid of that. Line count: 69,146, savings: 34 lines.

Okay, so I compile and run pieced_image_maker for the first time in years … there is an assert. It’s a dumb mistake where I used the wrong variable during a minor change I made the other day. Rebuild, run … and it runs! It sure goes a lot faster than it did back in the day.

Unfortunately if you don’t give it the command-line option not to do so, it writes its results into the hardcoded folder, which is /braid/game/run_tree/data/pieces, which is very convenient when you are working on the game all the time since it puts the output where the game wants it for the next run, thus clobbering the old output. But … the copy of Braid I am working in is in a different folder, since this is a massively disruptive refactor. What’s in /braid right now is supposed to be the safe copy … which I just clobbered with images of dubious safety. REVERT!!!

Trying again. The output looks reasonable, but it is definitely making some different decisions. So I’m going to have to figure out what to do here. First step will be to get an origin placed in the files somehow, and read that from the psd file code. But it’s time to take a break from Braid for a few days…