Month: June 2014

Just a short update here! I ran into some problems with the new Quaternion code, I’ll detail those in a later post. As part of getting that to work, I got sick of having to start the game over and over again. To alleviate this tedium, I implemented unit testing for the Actor methods related to the Quaternion work in the Grim Engine. Neat, but why was that interesting?

To get this working, I needed to solve a few problems:

• Some methods depend on a working, initialized GameEngine
• Linking all of the parts of ResidualVM into the test binary

In some of the methods, the engine depends on being initialized, with all of the correct settings. An obvious example of this is the check to see which game is being run by calling the getGameType() method. So, to fix this, I have written the minimum required code to start the main engine from the tests.

With that fixed, the next problem was that linking together the test binary was not as simple as before. If you just add the libgrim.a object, there will be quite a few missing objects in the final linked binary. To fix this, I found all of the .a files generated by the compiler and added them to the module.mk file for the testing build. Just adding them will not solve all of the dependency issues because the linker will search in order through the .a files to find the required symbols. If the objects containing those symbols haven’t been passed to the linker yet when encountered, the symbols will still be unresolved. So, you must order the objects so that all of the symbols can be found. Additionally, I added some extra code to the module.mk file so that when (for instance) support for Grim isn’t built and only Myst3 support is, the associated tests are not run.

I don’t think this will get submitted to ResidualVM, but it was an interesting exercise. This work can be found in my GitHub here. You must configure with –enable-text-console for this tree to work as it skips some GUI initialization that way.

Continued from the previous entry.

After a long hiatus, I’m working on completing the rotation and quaternion changes that arose as a part of fixing attaching and dettaching actors. To begin, I needed to rebase my old changes so that they applied to the current git master. With all of the progress that has been made since the start of GSoC, the patchset was out of date and doesn’t address all of the current uses of Quaternions and Rotation3d.

In order to make this patchset easier to accept into ResidualVM, I’m splitting the work up into three distinct parts:

1. Rework of Rotation3d to support the various Euler Angle orders
2. The final expanded Quaternion implementation
3. The completed attaching and detaching code

The reasoning behind this approach is that a large portion of the current Quaternion usage is simply to convert from Euler Angles to a Matrix. By removing this extra step, the resulting code will be easier to read and more straightforward. After applying my previous changes to the Rotation3d class, I created tests to check that the code was capable of converting from Euler Angles to a Rotation Matrix, and back again. In this implmentation, the user can specify the Euler Angle order by using the enum EulerOrder values, and building the Rotation Matrix with the member function buildFromXYZ or using the constructor that takes three angles and the EulerOrder.

Let’s take a brief detour here for a moment to explain how to add tests to ResidualVM’s test suite. ResidualVM uses CxxTest as the framework to build unit tests. All of the tests can be found in the directory test, with a directory inside that for each of the ResidualVM components tested. Currently, there are tests for the audio subsystem and the common functionality. In this changeset, I’ve created a new directory, math and added a new file, rotation3d.h which contains the unit tests for the Rotation3d class.

Unit tests are implemented by writing classes that contain the tests and CxxTest automatically generates the rest of the code to run the tests. So, to add a new test, we first include a header from CxxTest that defines the TestSuite class. We then define a class that inherits from this base class and then implement our tests with each unit test as a public member of our class. Each of these unit test functions return no value and have no parameters. To actually test a value, CxxTest provides assertions which should be used to report the test result.

Since math isn’t already a part of the testing structure, we’ll need to add a new directory to the unit test build system. To do this, you must add the test header files to the build by adding them to the module.mk file in the test root. So, to add our new math tests to the unit tests to be built, I modified the TEST variable with the text in red:

• TESTS := $(srcdir)/test/common/*.h $(srcdir)/test/audio/*.h $(srcdir)/test/math/*.h

I also needed to link the actual math library implementation to the unit test build, so I added the following to the TEST_LIBS variable:

• TEST_LIBS := audio/libaudio.a math/libmath.a common/libcommon.a

And that’s it! To build and run our tests, from your build directory, simply run:

• make test

The output will look like this (assuming I didn’t break anything!):

And failures will look like this when something does break, depending on the assertion type used:

And that’s it for testing! It’s very straightforward to add additional tests, so I’ll try to add code that performs testing in the future for applicable changes.

Back to Rotation3d! In this incarnation of the updated Rotation3d code, I decided to return the rotation direction to the “left handed” DirectX direction of rotation that was previously in Rotation3d. This will remove the necessity for all of the transpose() calls that I had previously needed to insert to produce the same result as before.

In addition to the basic assertions, I had previously created a test that compared the original implementation of Rotation3d to my new version for each of the uses in ResidualVM. I adapted these tests for the new implementation, but won’t submit them for inclusion because they duplicate a lot of code. The branch containing these tests can be found here, while the Rotation3d changes can be found in PR #936.

In the next post, we’ll look at the Quaternion implementation and finish working on attaching and dettaching.

In a previous post, I commented on a texturing problem with the lava in the scene lav. As I indicated in that post, while the work around that I submitted did fix the problem for the scenes with issues, the workaround itself was sort of a hack. I had assumed that if a texture was larger than 128 pixels on a side, then that texture should be clamped at the edges. Clamping prevents the OpenGL renderer from repeating the texture over the surface, resulting in lines when the textures don’t repeat cleanly. So why not just always use clamping? Well, in many cases we DO want the texture to repeat. Think of a typical grass pattern or a sandy beach texture, or in this case, the lava!

Although my approach did fix the problem, the solution was a hack. There’s nothing that really indicates that that is a cutoff for which approach to take. To investigate possible solutions, I used apitrace, a tool that records OpenGL state information, allowing you to replay, investigate and examine what OpenGL calls a program is executing to render the screen.

To capture a trace, we’ll use this command with the retail version:

• apitrace trace -o retail.trace wine ./Monkey4.exe -gl -w

And this with ResidualVM:

• apitrace trace -o residual.trace residualvm

Of note, to trace 32-bit Windows applications in wine, you will need a 32-bit version of apitrace.

To view the traces, I used the program qapitrace, a GUI viewer for replaying traces from apitrace. To run qapitrace on an existing trace:

• qapitrace retail.trace

With these traces in hand and in the viewer, I took a look at the size of the textures and when the glTexParameteri function was used on them in the retail version. To make the trace more manageable, I filtered the results for Texture Events using Trace->Options and configuring it like shown below.

After some investigation, I found that there were definitely textures that used clamping when the size was lower than 128 pixels and repeating when greater. Rats, that approach definitely was wrong!

It was at this point that Akz- suggested that I try clamping sprites, which are generally images painted on top of the scene (such as the candles’ flame or the Dainty at dock on Lucre Island) and repeat all other textures. This approach seemed to work well, also resolving all of the issues resolved by the previous patch. For this approach, I had originally thought of passing a texture parameter to the Material (which loads the texture) using an enum, but I ran into the issue of having to predeclare the enum when used in places that had also predeclared the Material class. In C++, unlike C, the enum has a variable storage size and isn’t assumed to be int. In C++11, there is support for predeclaring the type, but ResidualVM targets the C++03 standard, which doesn’t include this feature.

In the end, I realized that the retail version only used GL_CLAMP_TO_EDGE and GL_REPEAT, so instead of using an enum, I replaced it with a bool indicating that the texture should be clamped when true. The completed patch for this work can be found in PR #910. It fixes both the lava, as shown in the previous post, with a repeating texture, and the Dainty at the dock on Lucre Island, where the texture needed to be clamped to remove the lines as shown below. Note that in these pictures, they’re not exactly in the same place because the Dainty texture moves to simulate waves.

In the next post, I will get back to collisions and the actual lava puzzle issues.

On Monkey Island, there’s a puzzle involving a lava flume ride:

Unfortunately, the first thing that’s immediately obvious is that the lava texture is rendered improperly. After bisecting, it turns out that this is due to this recent commit to fix the lines in the intro bitmaps. After testing the retail version in a debugger, I found that it used both GL_CLAMP and GL_REPEAT. I’m not sure if the approach is correct, but it looked like only textures that were smaller than 128×128 used GL_REPEAT, while larger textures seemed to use GL_CLAMP. An implementation of this was submitted as PR #910. Here’s what the Lava Puzzle looks like now:

This puzzle is configured from the set lav, which starts a separate Lua script: lava_flume2.lua. This script contains the logic for building the puzzle and controlling the raft, separate from the normal game controls and logic. This is all contained in the variable flume. In this puzzle, the locations of the logs can be in 1 of 4 different orientations, appearing at different stopping locations. There are always four logs. In the game script, the paths that Guybrush might take are represented as a graph of connected nodes, with each node containing information as to whether the node is blocked with a log, the node’s coordinates, etc. As you can see in the screen shot above, Guybrush does not collide with the stuck logs, he just passes through them instead of bumping them into the lava stream.

So, why doesn’t Guybrush collide with the stuck logs? In the script, it appears that the collisions are detected by turning on the collision mode to COLLISION_SPHERE. This in turn calls into ResidualVM using the functions SetActorCollisionMode and SetActorCollisionScale, which configure collision detection between the actor and other actors. When these collision modes are active, the collision check between actors occurs in the ResidualVM function handeCollisionWith in actor.cpp. I commented this function to see what collisions were being checked and got no print outs during the lava puzzle. I did get some collisions with Timmy the Monkey on the beach in the set mib though, which provides an easier testing location than the lava puzzle and also displays broken behavior. In this scene, Timmy is randomly running around, including through the legs of Guybrush, when he should stop by colliding with Guybrush. Using a debugger, I discovered the call in the retail version for determining a collision and found that Timmy does register a collision whenever he’s close to Guybrush. In ResidualVM, this doesn’t happen. This is likely the root cause of the bug.

There are also other problems with the lava puzzle as well, such as the boat bouncing back and forth the second time through the puzzle, and being able to see through the bottom of Guybrush’s boat. The second problem is likely related to the issue with doors

Since this blog entry is getting long, in the next entries, I’ll discuss fixing these issues.

Continued from the previous entry

In the previous entry, we identified the reason why it’s hard to start a conversation with Herman Toothrot in the set toc. To debug this issue, I took a look at the set gpt because it has a variety of sectors, many with the same name. This results in sectors that are easy to move between, but transition in the wrong place. For example, the transition between gpt_village (sector 4) and gpt_starbuck (sector 5) happens at a different position in the retail version of EMI and ResidualVM, as shown below:

Let’s take a look at the sector map for the camera sectors for the set gpt:

In this picture, we can see that the two sectors overlap, resulting in the glitchy behavior we see in the example above. In the retail version, it appears that the higher ID sector takes precedence over a lower sector ID.

As another test, let’s move Guybrush into the sector marked 4 and check what the result of IsActorInSector(guybrush.hActor, “gpt_village”) is. We would expect this function to return “4” because this sector is named gpt_village, but in fact, we are told that Guybrush isn’t in the sector gpt_village. However, if we move Guybrush into the sector marked 16, which is another sector named gpt_village, we find that the same command returns 16, indicating that Guybrush is in the sector!

So, it appears that IsActorInSector() should only return the sector ID if the name matches the requested name and the ID is the highest of all of the sectors where the name matches. A patch that implements this is found in an update to PR #883.

When Guybrush enters Herman Toothrot’s camp (the set toc) for the first time, Guybrush should be able to trigger a conversation with Herman by walking up towards him. In ResidualVM, this conversation is never triggered! Let’s find out why.

In this scene, there’s a function to control the setup changes, including the closeup for the conversation Herman. This function checks if Guybrush is in the setup toc_log and if the conversation has already happened. So, why doesn’t the standard “Talk to Herman” come up when we approach? In this script, the range for this pop-up is reduced to 0, which prevents it from coming up. Setting it with:

• toc.herman_obj.range = 4.3

Makes this behavior work as intended, and indeed, the script does this after Guybrush has the initial conversation with Herman. We can also check the state of the conversation, just to make sure this isn’t the problem either (using my pop up lua injector):

• dd = glob.herman_talk_status

And as expected, it has the correct value, 0. Let’s check Guybrush’s current setup now:

• dd = toc:current_setup()

Which returns 1, the ID for toc_wide. So it seems that Guybrush never enters the setup toc_log, which is required to trigger the conversation.

First of all, are we sure that the setup_change function is being run? Yes! We can prove this by checking the range on Herman’s note in this scene. If we check from the setup toc_wide, the interaction range for the note is 0, but when we change to the setup toc_close, the interaction range is changed to 1.8. This change is implemented in the setup_change function, so it’s certainly being run.

How about if we force the current setup to toc_log?

• toc:current_setup(toc_log)

That works, and we’re treated to the conversation with Herman. So, it seems that Guybrush isn’t moving into the setup toc_log. As with the previous bug at Pegnose Pete’s House, there is a problem moving between setups, but this time, it’s impossible to move into the setup toc_log.

Let’s check to see what sector Guybrush is in with find_sector_type:

• system.currentActor:find_sector_type(CAMERA)

We find that the sector reported when we’re near the log is actually toc_log.

How about IsActorInSector()? Both toc_wide and toc_log both are matched as valid setups when Guybrush is near the log. However, in the retail version, only toc_log matches, with IsActorInSector(ActorHandle, “toc_wide”) returning a nil, even when the setup toc_wide is showing! Trying this test again with the set gpt also resulted in nil values for IsActorInSector when in setups gtp_arial and gpt_dock, while ResidualVM reported that Guybrush was in the sectors of the same name.

First, let’s see if the sectors actually overlap. To visualize the sectors in a set, I rewrote a utility in perl from residualvm-tools to work with EMI and GRIM. This new version can be found here. The input of this tool is a text based set (so you’ll have to convert binary sets with setb2set first), and the output is a fig file, compatible with xfig. I also added the ability to filter by sector type.

For the set toc, I filtered by type camera, which is the type used to pick the view and got the following result:

From this, we can easily see that the sector toc_log (sector 19), overlaps one of the toc_wide (sector 0) sectors. After going through all of the sets, I found that this set is the only set in which the camera sector is almost completely overlapped, aside from the set cpt. In the set cpt, the two overlapping sectors have the same name.

So, why does this bug arise? When the camera change function is called, it checks to see if Guybrush is still in the same sector by calling IsActorInSector(). This call succeeds when it should fail because the sectors are overlapped, so the camera change function thinks Guybrush hasn’t left the sector.

This post is going on a bit long, so in the next post, we’ll come up with a solution to this problem.

Continued from the previous entry