Wednesday, September 14, 2011

Living without client

Recently I've written about the possibility to test the features by writing code, that test them. As I've started doing some refactoring in our code, I immediately wanted to use that feature. Let's see how it worked out...

Gathering, once again


As we already mentioned, we are changing (again) the way resources will be gathered. This of course required yet-another-refactor of that part of code. Last time I was doing this, it was rather boring task: refactor script of one facility, start server, login, spawn some of those facilities, spawn myself needed item, use it, check. Rinse and repeat (sometimes you may hot-reload the script, but you need to still test it manually).

This time I decided to write set of test cases for every 'use event' that results in resource being acquired. And then, those tests will check the stuff for me, so that I do not even need to fire up the client. Nifty!

class Plant : IFacility
{
Item@ item;
uint16 resource;
uint batch;

Plant(Item& item, uin16 resource, uint batch)
{
item.SetEvent(ITEM_EVENT_USE_ON_ME, "_UseItem");
item.SetEvent(ITEM_EVENT_SKILL, "_UseSkill");
item.SetEvent(ITEM_EVENT_FINISH, "_Finish");

@this.item = item;
this.batch = batch;
this.resource = resource;
this.batch = batch;
}

int get_Amount() { return item.Val1; }
void set_Amount(int val) { item.Val1 = val; }

bool UseItem(Critter& cr, Item& usedItem)
{
uint pid = usedItem.GetProtoId();
if(pid == PID_KNIFE || pid == PID_COMBAT_KNIFE || pid == PID_LIL_JESUS_WEAPON || pid == PID_THROWING_KNIFE)
{
if(IsOverweighted(cr))
{
cr.SayMsg(SAY_NETMSG, TEXTMSG_GAME, STR_OVERWEIGHT);
return true;
}
else
{
cr.AddItem(resource, batch);
cr.SayMsg(SAY_NETMSG, TEXTMSG_TEXT, text);
}
return true;
}
else return false;
}
}


This is an excerpt from a class that represents plant facility that we may cut with knife to use the needed resources. It's represented in-game as an item, and we are initializing it in item script in following way:
void item_init(Item& item, bool firstTime)
{
AddFacility(item, Plant(item, PID_FIBER, 2));
}

It uses the pattern I've described in following post. The Plant constructor assigns the events to the item:
// critter uses skill on facility
bool _UseSkill(Item& item, Critter& cr, int skill)
{
IFacility@ facility = GetFacility(item);
if(valid(facility)) return facility.UseSkill(cr, skill);
return false;
}
// critter uses item on facility
bool _UseItem(Item& item, Critter& cr, Item@ usedItem)
{
if(valid(usedItem))
{
IFacility@ facility = GetFacility(item);
if(valid(facility)) return facility.UseItem(cr, usedItem);
}
return false;
}
// :>
void _Finish(Item& item, bool deleted)
{
IFacility@ facility = GetFacility(item);
if(valid(facility)) RemoveFacility(item);
}


Testing stuff


First thing we would probably like to test is, whether critter really receives the items if using proper tool on that facility.Let's first prepare a helper that's gonna greatly reduce the amount of repeated code:
void mock_CritterAddItem(Critter& cr, uint16 pid, uint count)
{
CallExpectation("CritterAddItem_" + pid + "_" + count);
}

class Fixture
{
Critter@ cr;
Item@ tool;
Item@ item;

Fixture(uint16 tool, string@ script)
{
@cr = MockCritter(1);
Mock("Critter::AddItem", "mock_CritterAddItem");
@this.tool = MockItem(tool);
@this.item = MockItem(1); // this pid does not matter
item.SetScript(script);
}

IFacility@ get_Facility() { return GetFacility(item); }
}

And now, the first test:
void test_PlantFruitGivesProperResource()
{
Fixture fix(PID_KNIFE, "prod_plant_fiber@item_init");
fix.Facility.Amount = 100;
ExpectOnce("CritterAddItem_" + PID_FIBER + "_" + 2);

fix.item.EventUseOnMe(fix.cr, fix.tool);

VerifyExpectations();
}

The most important part is the mock for Critter::AddItem, which just stores the expectation named in following manner: CritterAddItem_PID_COUNT. In above test function, we're preparing a context consisting of:
  • critter
  • knife
  • item representing the plant
We invoke the event that would be normally invoked when player or npc would use a knife on the plant (EventUseOnMe). The call process normal behavior, but when it encounters call to Critter:AddItem it calls our mock instead. So we expect that our function is gonna call this function with PID_FIBER and 2 as parameters. Looking into the code of Plant::UseItem, we see that indeed it should work. Plant::batch is equal to 2, so it's gonna execute exactly what we wanted. Running the test confirms that. Yay!

Code is already bugged


Oh great:( I'm writing some code snippets that already contain bugs. So what? Let's fix them, but first let's write a test that confirms them, check if it fails, and then fix the bug and re-check the test:
void test_PlantFruitEmpty()
{
Fixture fix(PID_KNIFE, "prod_plant_fiber@item_init");
fix.Facility.Amount = 0;
ExpectNonce("CritterAddItem_" + PID_FIBER);

fix.item.EventSkill(fix.cr, -1);

VerifyExpectations();
}

Since there are gonna be no timeouts on characters, the facilities itself should somehow limit the amount of resources that we can obtain (and they will be regenerated over time). We've already had implemented some Amount property in our Plant class, but the code didn't care about it. And this test proves that - we expect that AddItem is not gonna be called with PID_FIBER (and whatever count) at all, but it is, because we do not check for amount left in UseItem method. Let's fix it:

            // ...
else
{
if(this.Amount > 0)
{
cr.AddItem(resource, batch);
cr.SayMsg(SAY_NETMSG, TEXTMSG_TEXT, text);
}
}
// ...

And let's run the test again. It succeeds! We've fixed a bug, we tested it. Without doing manual labor.

The method I'm describing here, really makes me happy developer, as I do not need to spend time on manual testing, and also I am preparing more and more tests that's gonna test whether existing features are still working in the future (you know, some change in other area of code can really break the other part of code, in an unexpected way, better catch at least some of such bugs).

There is still one bug left in the code (probably much more, I wrote this post without compiler). I'm leaving it for readers to find out and write test code that proves it!

Friday, September 9, 2011

Walking the way of bombs

It isn't exactly news that various towns in the wasteland are racked with the crazy nature of suicide bombers. In real life and in FOnline: 2238. This particular way of entertainment is in almost all cases only entertaining for a very small minority of people and due to this, discussions internally and externally are kind of an unofficial sporting event by now.

Anyway, to make it short: In the future we want to change the way explosives are working and used in the game, to limit (but not totally remove) the amount of suicide runs. To accomplish this, we internally agreed on the following ideas:
  • Guards can "sniff" activated explosives (C4, Dynamite, and placed Landmines).
  • The player can't activate explosives (C4, Dynamite, and placing Landmines) whenever a guard is around or if the player is in guard range. (It simply doesn't work, with an entry in the message log.)
  • The player can activate explosives (C4, Dynamite and placing Landmines on the ground) whenever a guard is not around or if the player is not in guard range.
The few rules should result in harder suicide runs, without making them impossible, because the players action now will be as the following:
  1. The PC has to activate the explosives far away from guard NPCs,
  2. then the PC has to run up to their targets (NCR bazaar....),
  3. and then hopefully explode there before a guard can kill the bomber and take the explosives (disarming them).
These simple changes should make the game better than with the current system (guard NPCs attack players with active explosives), which is pretty much useless, as especially dynamite can be triggered in around one second and nobody is able to react on it this fast.

Of course, the subject is still open for discussion and suggestions in the forum (for example, there were ideas about involving demolition expert profession in this process). Also we aren't quite sure about when exactly we are able to take this over into the game. It probably won't happen before the next wipe, but I'll hope we can push it afterwards.

Tuesday, September 6, 2011

Testing your server scripts

I've finally gotten around the way to unit test FOnline scripts. I'm happy about it, though it's still a prototype, and I've only written few tests just to try it out.

Why test your code and why should we care? Well, it's very popular approach nowadays, to write a code that tests your code, just to be sure everything works as expected (and it's hard to keep track of everything with large codebase). It's common misconception that's not feasible for games, after all, how can you test something as complex as interactive program - and even worse - with multiple players?

The answer is that you're not writing a code to test your whole game. You're not gonna write a bot that plays it, does all the quests and check if everything still works (and even looks for holes in the walls, though that'd actually be cool). You write the code to test only portions of it, the small pieces, the units. Hence the name: unit testing. There's of course lots of materials on it over the internet, so we're not going to dive into the details, I just describe what were the problems, and how we solved it, so that we can finally try unit tests.

Isolating the code


When you look at the code you'd like to test, first thing that come to mind is that it's probably too complex and involves too much dependencies to be easily testable. Let's look at the example:
bool critter_use_item(Critter& cr, Item& item, Critter@ targetCr, Item@ targetItem, Scenery@ targetScen, uint param)
{
bool isPlayer=cr.IsPlayer();
bool useOnSelf=(not valid(targetCr) && not valid(targetItem) && not valid(targetScen))

// Radio
if(FLAG(item.Flags,ITEM_RADIO) && useOnSelf)
{
if(isPlayer) EditRadioSettings(cr,item);
return true;
}
// ...
}

Say we need to test that when player uses the radio item, he can edit its settings. Unfortunately, to invoke critter_use_skill int this context we need:
  • player
  • item
OK, we may easily spawn radio, but how can we spawn player? Scripting subsystem does not allow us to do this. We could just spawn critter, but then we see script cares about IsPlayer() condition. And then, how do we test whether player actually saw the edit radio interface or not? There is that elusive EditRadioSettings function being invoked there, maybe we could somehow detect that it's been called? But how?
Due to above limitations, unit testing your code seems just not feasible. Surely, there must be some solutions, we just need to go out and learn how the world is doing this.

Mocks


The concept is simple. If you need to provide something for the tested code, that's not the part of the test itself but constitutes to the test context, provide a mock. A substitute for real object, a substitute for a function that's gonna allow us to isolate the tested code to the form that may be safely run from within unit test context. As far as above example goes, we would need three mocks:
  • player - Critter object with overriden IsPlayer() method that's returning true (another mock actually), or with fields set in a way that IsPlayer() is returning true
  • an item - Item object with fields telling us it's a radio
  • a function to detect that EditRadioSettings has been called
But we can't do this in AngelScript. Not without any modifications.

Mock library

At first I wanted to write some library that would be able to load the script, compile it and execute test functions providing overriden implementations for functions we wanted to mock. However, the AngelScript engine does not allow to re-register any function, so that once we've got our engine set up, we can't register any mocks. We thought of some workarounds, but after some time I decided to modify the server source directly (its angelscript source, to be precise), I suspected it would be very minor modification - so that maintain costs are minimal in the future (keeping it up to date with every server update).

Tests Runner server

After some fiddling in AngelScript engine code, I found out that I can easily 'redirect' function call to another script function (whether the original call was meant to invoke script function, or native (engine registered) one. Moreover, it turned out that method calls can be simply redirected to function calls (providing the first argument is the object passed), without any extra work! This way call to bool Critter::IsDead() could be handled by bool critter_IsDead(Critter& cr). Nice!
How does it affect our testing capabilities? To put it simply, we ended up with solution that may be used for testing generally, not only for unit tests. Unit tests are designed to test your small pieces of code out there, but hence we are running our tests in full fledged server, we may as well test the system more broadly, we may check how different pieces interact together - whether it works as a whole or not (though I admit, it wasn't my initial goal - I just wanted simple unit testing facility, not integration tests).

But let's get back to the example. Let's start from the last mock we needed, namely, a function substitute for EditRadioSettings. Say, we just want to know, whether the function has been called or not. Let's mock it, and make it so our substitute will indicate that's been called:
void mock_EditRadioSettings()
{
CallExpectation("EditRadioSettings");
}

For above example to work, we need to know what CallExpectation function is doing. It simply increases the call counter, that's stored in some dictionary under the index that's been passed as argument. Later on, we may check that the counter is equal to 1 - that means our function has been called as expected. With such dictionary at hand, we should also define more helpers: Expect(funcname) and VerifyExpectations(). First one is used to remember the fact, that we want the function funcname to be called, and the latter (called at the end of the test), will check it. In fact, we should have the ability to specify the numbers of the call we're expecting:
  • ExpectOnce(funcname) - VerifyExpectations will succeed only if function has been called once
  • Expect(funcname, count) - success only if called count times
  • ExpectNonce(funcname) - success if hasn't been called at all
Once we've got our mock and our helpers ready, we're gonna substitute our real function:
Mock("EditRadioSettings", "mock_EditRadioSettings")

This function will remember that we want to redirect the call to EditRadioSettings to a function mock_EditRadioSettings.

Ok, what about other mocks? We said that we want an item, and a critter with specific properties and/or we may provide mocks for their methods as well. For this I've implemented simple MockCritter and MockItem functions that spawn the needed objects with only basic properties filled, but for the case of this example, let's assume those are just clean objects - with all properties zeroed out, rest will be handled by method mocks:
bool mock_True()
{
return true;
}


And now, our test function (with needed mocks) in full glory:
bool mock_True()
{
return true;
}
void mock_EditRadioSettings()
{
CallExpectation("EditRadioSettings");
}

void IfPlayerUsesRadioEditRadioSettingsShouldBeCalled()
{
// prepare
Critter@ player = MockCritter();
Mock("Critter::IsPlayer", "mock_True");
Item@ radio = MockItem();
radio.Flags = ITEM_RADIO;
Mock("EditRadioSettings", "mock_EditRadioSettings");

// call tested function
critter_use_item(cr, radio, null, null, null, 0);

// verify
VerifyExpectations();
}

Voila! Now, when we call critter_use_item, it will first call Critter::IsPlayer(), which returns true, even if our Critter structure might have not indicated this, but our mock did. Later, it will check the flags (we've set it on our mocked item) and call the EditRadioSettings, which in fact calls mock_EditRadioSettings. This sets our expectations counter to 1, which is then verified by VerifyExpectations. And then it announces success - we've got our first trivial unit test passed!

The solution we've got is in very early stage, I'm gonna try and involve it in some of the 2238 code testing. We will see if it turns out to be useful.

Enough of this mockery for now!