Progress Report (End?) – a-yyg's blog

The coding phase of GSoC is basically over, so this will be the last GSoC-related progress post. We’ll be going over what has progressed since the last post and what challenges are left to face post-GSoC. With that in mind, the next post will be my actual last GSoC post.

Playtesting

Playtesting is over! The ride was surprisingly smooth until around 50-60% of the game in, but I experienced a few issues during the second half. Here are the additional bugs and the way I fixed them:

Bug	Steps to reproduce	Explanation	Solved
Actors loading with the wrong appearance	?	Sometimes some actors seem to load with mismatched appearance (i.e., Frost Giant as Flame Giant, etc.)	No
Return key not working on save dialog		The return key should save the file and close the dialog when pressed. This does not happen.	Yes
The captain does not sail you to Tamnath Ruins	Defeat Maldavith and then talk to Captain Navis.	After killing Muybridge in the cave below Maldavith, Captain Navis should allow you passage to the Tamnath Ruins through his ship. This does not happen.	Yes?
Crash when turning the lever inside Tamnath Ruins	Turn the leftmost lever inside the rightmost building.	Three levers need to be operated to get one of the Tapstones. One of these lavers causes the game to crash when interacted with.	Yes
Audio crash in Underworld	Walk through the plane behind the huge door blocking passage to Sariloth’s place.	Walking through the passage that leads to Sariloth causes a crash.	Yes
Timer Save error	?	Error when saving	No
Some keys not working on document and automap		Arrow keys should control the pages when reading a document, and Home/End/PgUp/PgDn should control the automap.	Yes

Return Key Not Working on Save Dialog

It was simply a matter of if/else if logic order not making sense.

Fix: Rearrange the conditionals.

https://github.com/scummvm/scummvm/commit/079330547048d11ab0151ef6e100d66e3902b96a

Captain Does Not Sail You

This was (and still is) a mystery. I spent an entire day tracing the scripts to figure out what was going wrong, but this requires even further digging.

I’ll explain briefly how the dialogue script flow goes:

When you double click on an actor, the method Actor::greetActor is run. This, in turn, causes a runObjectMethod call that calls some scripts, which then calls Actor::useKnowledge. This is illustrated in the backtrace below.

#0  Saga2::Actor::useKnowledge(Saga2::scriptCallFrame&) (this=0x6210007375b8, scf=...)
    at engines/saga2/actor.cpp:3180
#1  0x0000555555f33499 in Saga2::scriptActorUseKnowledge(short*) () at engines/saga2/sagafunc.cpp:1993
#2  0x0000555555e23b1d in Saga2::Thread::interpret() (this=0x60b00005bc10) at engines/saga2/interp.cpp:877
#3  0x0000555555e2b601 in Saga2::Thread::run() (this=0x60b00005bc10) at engines/saga2/interp.cpp:1653
#4  0x0000555555e2ce43 in Saga2::runMethod(unsigned short, short, unsigned short, unsigned short, Saga2::scri
ptCallFrame&) (scriptClassID=13, bType=-1, index=32800, methodNum=136, args=...)
    at engines/saga2/interp.cpp:1875
#5  0x0000555555e2d017 in Saga2::runObjectMethod(unsigned short, unsigned short, Saga2::scriptCallFrame&)
    (id=32800, methodNum=136, args=...) at engines/saga2/interp.cpp:1897
#6  0x0000555555d7fa5e in Saga2::ActorProto::greetActor(unsigned short, unsigned short)
    (this=0x608000044220, dObj=32800, enactor=32768) at engines/saga2/actor.cpp:355

Actor::useKnowledge decides which dialogue response to give and calls the speech methods, again, through SAGA scripts.

res = runMethod(knowledge[i],
                builtinAbstract,
                0,
                Method_KnowledgePackage_evalResponse,
                scf);

The above code is a call to evaluate the given knowledge to decide which response to give. This is where the problem starts. The script that is called with knowledge[2] for Captain Navis has a conditional branch within it. Depending on the result of the conditional, the resulting dialogue changes.

Here is a snippet of the script execution log:

[0241 0x00f1]: op_getint
IMMED_WORD(130 0x0082)
IMMED_WORD(2862 0x0b2e)
byteAddress: far[130:2862] = 0
stack size: 3: [0 -32655 512 ]
[0247 0x00f7]: op_jmp_true_v    <-- if (getint(130, 2862)) jmp

The jmp instruction in the last line is the culprit. It takes in the leftmost (most recent) value in the stack, and if it is different than 0, it jumps. Here, the leftmost value is 0, so we do not branch out. However, if we actually take the true branch, the correct dialogue option appears.

To figure out the reason for this miss, we’d have to dig further. Since we do not have that much time to waste, I decided to just implement a workaround for this particular issue since I didn’t experience any problem anywhere else.

Fix: Implement workaround for method byteAddress when seg = 130 && offset == 2862.

https://github.com/scummvm/scummvm/commit/876126f7b83ab8ae658b8278a6cf44128002e757

Tamnath Ruins Lever Crash

Here we had some nullptr accesses combined with uninitialized wild pointer accesses, so I just added checks.

Fix: Add nullptr checks and initialize pointers.

https://github.com/scummvm/scummvm/commit/29c5f6e7494d46fa0b927507251d150b8b7879d6

https://github.com/scummvm/scummvm/commit/fb15fce9d68c52a500a0832d2ee4419a83ecdc6d

Audio Crash in Underworld

Here a track beyond our track limit was attempted to be played, resulting in a heap buffer overflow which ASAN detected. This would likely not result in anything significant for a normal playthrough (which is why it remained dormant so far).

Fix: Add checks around the track number in audioEnvironmentCheck.

https://github.com/scummvm/scummvm/commit/87b1f858e5eddf3028ee8958074723c038fbdd69

Document/Automap Keys

There were still instances of the original keyboard detection method SpecialKey being used, so I replaced those.

Fix: Replace SpecialKey by Common::KeyState.

https://github.com/scummvm/scummvm/commit/81909e6dabffe56949ff410c4c387aea4b71934a

I fixed quite a few of the bugs we had discovered previously as well.

Tooltip Clipping Off-screen

This was the result of g_vm->_pointer‘s internal parameters not being adjusted properly.

Fix: Call gPointer::move properly.

https://github.com/scummvm/scummvm/commit/9e894a82d63a5a69c299c4a620e63b182b87c607

Staff Of Life Crash

The SAGA Script for the Staff Of Life called the method Object2Actor, which returned the ObjectID of the object if it was an actor, or 0 otherwise. This causes an error because the 0th object is a dummy without scripts.

Fix: Make the Object2Actor method always return the ObjectID.

https://github.com/scummvm/scummvm/commit/0e6813338292fd0494eb2cc4f6b7dc4d7333bb38

Midair Item Interaction Crash

When an object is in midair, it has no parents, so interacting with it in this state causes a crash.

Fix: Change the getMapNum method to return the sibling’s parentID if the current one has no parent.

https://github.com/scummvm/scummvm/commit/974d4dd81eb92c7db1c66928dd841241b6ae0264

Book Text Missing Last Character

This was due to a wrong strlycpy range in document.cpp.

Fix: Use the correct range for strlcpy.

https://github.com/scummvm/scummvm/commit/0bdf49fb71baac25f8c9856cdf55ce3127271e5a

Music Not Looping

The flag for music looping was not ever actually set. Is this due to differences in the binary executable distributed to us and the actual source we got?

Fix: Set the loop flag to true.

https://github.com/scummvm/scummvm/commit/994ac18c52785e69213e2312f435f73fa76ef75b

SpellID Saves

Previously we discussed how I would have to find out the correct size for enums, but then never actually did. Turns out that is actually necessary since saving with spells present brings out compatibility issues with the original.

Fix: Change saved SpellID size from 4 bytes to 1 byte.

https://github.com/scummvm/scummvm/commit/aac41f36f8347c94ce06c0505c6ca138f3c2331b

Console Commands

I added two new debug commands, as well as extended an existing one:

// Input: <1/0>. Sets whether you can teleport by right clicking on the screen.
bool cmdTeleportOnClick(int argc, const char **argv);

// Input: <1/0>. Sets whether you can teleport by clicking on the map.
bool cmdTeleportOnMap(int argc, const char **argv);

// Input: <1/0>. Sets the invisibility effect on the party.
bool cmdInvisibility(int argc, const char **argv);

TeleportOnClick now teleports the player with the right click (so we don’t have to continuously turn it on and off). Also, shift + right-click teleport the entire party.

TeleportOnMap causes the player to teleport to where they clicked on the map.

Invisibility sets the player characters invisible. Great for teleporting around the map without engaging in battle!

What is left?

I believe it is safe to say that most of the engine has complete functionality. However, work is still far from over in SAGA2. I will expand on this in a future post detailing the project’s progress as a whole and what needs to be done. However, a few examples are: code refactoring to comply with ScummVM’s naming conventions; playtesting on devices of different architecture; fixing existing issues that have only been remedied, like the scripts and further debugging of tasks.

This was a fun ride, and I learned a whole ton. I am extremely grateful to my mentor and the other members of the ScummVM community. Although I can’t make any concrete promises, I plan on continuing to update SAGA2 outside of GSoC in my free time when possible since I believe this engine has much more to offer.