Week ∞+1=?

Let’s talk about last week’s developments.

Audio!

So this week we started implementing audio. Their original audio interface had various peculiarities, such as a number of audio decoders. I won’t waste time detailing them here since we’ve scraped a good part of it (thanks to OSystem’s Audio interface!). I mostly focused on the code refactoring and let sev deal with the actual audio decoding, since he is definitely more knowledgeable than me in this subject.

The first development was enabling speech and organizing it using Common::Queue (the original used a custom queue class that we scraped). The good news is that after some fiddling we were able to hear (high-pitched) voices! Looks like audio resource loading is working properly. The other good news is that this fixed Kwarrel’s heart-attack induced death we discussed last week.

So it turns out there is a routine that checks whether a certain dialogue is finished:

bool Speech::longEnough(void) {
	if (speechFlags & spHasVoice)
		return (!stillDoingVoice(sampleID[0]));
	else
		return (selectedButton != 0 || speechFinished.check());
}

We theorized that due to the audio failing to be played, it messed up his script sequence, resulting in an early death.

In other news, very recently sev has implemented music playing, so the game is not a nihilistic void with occasional voices anymore. Nice!

A bug that has sprung up is that speech texts now disappear very rapidly. My personal hypothesis is that this could be due to an error in the routine that determines which dialogue is currently being played, because as you can see from the following debug snippet, some speeches are processed very quickly (H920).

WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: stillDoingVoice(H919)!
WARNING: STUB: audioInterface::queueVoice(soundSegment [], sampleLocation)!
WARNING: STUB: stillDoingVoice(H920)!
WARNING: STUB: stillDoingVoice(H920)!
WARNING: STUB: audioInterface::PlayMe()!
WARNING: STUB: audioInterface::queueVoice(soundSegment [], sampleLocation)!
WARNING: STUB: stillDoingVoice(H921)!
WARNING: STUB: stillDoingVoice(H921)!
WARNING: STUB: stillDoingVoice(H921)!
WARNING: STUB: stillDoingVoice(H921)!
WARNING: STUB: stillDoingVoice(H921)!
WARNING: STUB: stillDoingVoice(H921)!

Global Constructor Hell

I probably spent the most time this week dealing with global constructors. The code is riddled with them, and we should remove all of them if possible.
But first, I should explain what are global constructors and why they are dangerous.

Now, let’s say creates the following code in C++:

#include <cstdio>

int c;

int main() {
  c = 1;
  printf("%d", c);

  return 0;
}

This is fairly standard global variable declaration and initialization. I’m sure we’ve all done something similar when we were writing our hello worlds in C/C++. Does this call a global constructor? No, because integer types (and more generally, plain-old data types) do not have a constructor.
Now suppose you’d like to write code like this:

#include <cstdio>

class Integer {
public:
  int num;
  Integer(int num_ = 0) : num(num_) {}
};

Integer c(1);

int main() {
  printf("%d", c.num);

  return 0;
}

Your code compiles and it outputs a 1. Great, we’re using constructors and everything works.

Let’s modify the above code a bit:

#include <cstdio>

class Integer {
public:
  int num;
  Integer(int num_ = 0) : num(num_) {}
};

class IntIsPair {
public:
  int pair;
  IntIsPair(Integer num) {
	  if (num.num % 2 == 0)
		  pair = 1;
	  else
		  pair = 0;
  }
};

Integer c(1);
IntIsPair a(c);

int main() {
  printf("%d", a.pair);

  return 0;
}

Again, the code compiles and it prints out a 0, because 1 is not pair. Sweet! Now what happens if we separate these two classes into two different source files?

integer.h:

class Integer {
public:
  int num;
  Integer(int num_ = 0) : num(num_) {}
};

extern Integer c;

integer.cpp:

#include "integer.h"

Integer c(1);

main.cpp:

#include <cstdio>
#include "integer.h"

class IntIsPair {
public:
  int pair;
  IntIsPair(Integer num) {
	  if (num.num % 2 == 0)
		  pair = 1;
	  else
		  pair = 0;
  }
};

IntIsPair p(c);

int main() {
  printf("%d %d", p._pair, c.num);

  return 0;
}

Strange, now it outputs “1 1”, as if 1 were pair…
The reason for weird behavior is that the order of static/global objects’ construction is not well defined between different translation units (source files). This means that c may not be constructed by the time p is constructed, resulting in many possible problems.

Because of this, we have to get rid of as many global constructors (which are warned with clang) as possible. To fix this, we have mainly two options:

1. Turn the class into a POD (Plain-Old Data) struct without constructors. For instance, in the code below, instead of Rect16…

class Rect16 {
public:
	int16               x, y,
	                    width, height;

	// constructors
	Rect16() { x = y = width = height = 0; }
...
};

We may define a struct StaticRect

struct StaticRect {
	int16 x, y, width, height;
};

And initialize it like a normal struct.

Or, you may instead…

2. Define it within a class (like Saga2Engine, the engine’s main class) and initialize in the constructor (or another similar place).

Saga2Engine::Saga2Engineclass Saga2Engine : public Engine {
public:
	Saga2Engine(OSystem *syst);
	~Saga2Engine();
...
        Rect16 _someRect;

Saga2Engine::Saga2Engine(OSystem *syst)
	: Engine(syst) {
	const Common::FSNode gameDataDir(ConfMan.get("path"));
...
        _someRect = Rect16(-1, -1);

There were originally about 160 global constructor warnings, but we sit at 69 at this moment in time. Well, progress is progress!