Since the last post I’ve worked on the screenshots feature. There was few requirements:
Last 2 weeks I’ve worked on developing of format for holding recorded gameplay data. Why this needed and how does it work you may read under the cut.
In first versions I wrote without formatting and then just read it in same order to the written. It worked, but wasn’t flexible and hadn’t any abilities to provide backward compatiblity in future. And there was a problem with navigation between file sections: for example, file has a header section, a section with info about autor and a section with gameplay data.
So, it was necessary to find the way to structure the data. I thought that the most appropriate format for current purposes is the RIFF, i.e. use containers with fields of unique identifier and size for every file section. It give us opportunity to find identify any section in file. Also we can easily skip any unsupported section.
Now file has following sections:
PBCK //format id, length = file length - 8
VERS //version of recorder
HEAD //Header, contains unnecessary info about author
HAUT //Author's name
HCMT //Comments
HASH //table of MD5 hash of game files
HRCD //record for one file
HNAM //name
HMD5 //MD5 hash
RNDM //Seeds for random number generator
SETT //Game's settings
SKEY //Name of setting
SVAL //Value of setting
RCDS //Gameplay data
RCDS section always has 0 in size field, because isn't possible to calculate it during the recording.
To process this file, I wrote state machine which may be illustrated by following diagramm:
Hi. Last week was pretty fruitful. I got to work EventRecorder on KYRA engine. Let’s describe it in more details.
Hi to all. It’s not my first record in this blog, but I had no chance to introduce myself in previous posts. In this post I’m going to fix this unfortunate mistake.
My name’s Danil. I’m ukrainian student and am studying computer science in university. You might have guessed on the basis of my project’s choice, but just in case I will specify that I really love computer games.
Now about news. Of course, main news is MY GSOC PROPOSAL HAS BEEN ACCEPTED!!! What does it mean in practical plane? In practical plane it mean that during the next 4 month you’ll read in this blog how I’m writing “Testing framework for ScummVM“. I promise that will do my best to make this blog not very boring to read.
What can I say else? Fortunatelly, in this year Ukraine choosed for EURO2012 soccer champ. Our goverment pay a big attention to this event, therefore my university’s exams begins and ends early, so I’m almost pass them and ready to work right now.
Maybe it’s all that I can say in introduction. So see you later in next post after I do some work and be able describe any interesting technical details
I tried to make something workable in time, but it was much harder than expected. So I’ll show what I had done for this moment. Sources you can see at this LINK. There I’ll tell how and what I did to realize this idea.
I’ve read and criticism of my idea to link events with engine’s loop. I’ve desided that it’s realy hard in realization and impossible to make universal for all existing engines. Then I’ve desided to write every “external” event (with some exceptions), namely mouse’s moves and clicks, keyboard’s events, calling of delay and getmillis functions.
I’ve quite quickly writtem it, fortunately most of all this was already in existing code. Then I’ve tested it on SKY engine and everything worked perfect.
In my first post I’ve wrote about refactoring of SKY engine. Here I want to describe it in more details what did I mean.
Let’s look to the main engine’s loop. It’s huge, so I’ll process it part by part
First thing we’re seeing – nested pause loop. Are we really need it? Or may remove without any regrets? Obviously it’s paused state of engine. When engine in this state, it’s only make delay and check keyboard for ‘P’ button pressed. (Also there is one moment. In this state user can’t close game cause shouldQuit() procedure doesn’t processed. I don’t know actually is it bug or feature, but seems like bug)
while (!shouldQuit()) {
_debugger->onFrame();
if (shouldPerformAutoSave(_lastSaveTime)) {
if (_skyControl->loadSaveAllowed()) {
_lastSaveTime = _system->getMillis();
_skyControl->doAutoSave();
} else
_lastSaveTime += 30 * 1000; // try again in 30 secs
}
_skySound->checkFxQueue();
_skyMouse->mouseEngine();
handleKey();
if (_systemVars.paused) {
do {
_system->updateScreen();
delay(50);
handleKey();
} while (_systemVars.paused);
delayCount = _system->getMillis();
}
After I removed nested loop, code begin to show that the pause is a state of loop. Also removing of totally useless loop helped us to make code more readable
while (!shouldQuit()) {
if (_systemVars.paused) {
_system->updateScreen();
delay(50);
handleKey();
delayCount = _system->getMillis();
}
else {
_debugger->onFrame();
if (shouldPerformAutoSave(_lastSaveTime)) {
if (_skyControl->loadSaveAllowed()) {
_lastSaveTime = _system->getMillis();
_skyControl->doAutoSave();
} else
_lastSaveTime += 30 * 1000; // try again in 30 secs
}
Now let’s pay our attention to the bottom of go() function. There is calculation of delay
if (_fastMode & 2)
delay(0);
else if (_fastMode & 1)
delay(10);
else {
delayCount += _systemVars.gameSpeed;
int needDelay = delayCount - (int)_system->getMillis();
if ((needDelay < 0) || (needDelay > _systemVars.gameSpeed)) {
needDelay = 0;
delayCount = _system->getMillis();
}
delay(needDelay);
}
I want to move it to separate function because it make reading of main loop much harder than it can be.
int SkyEngine::calculateDelay( uint32 &delayCount ) {
if (_fastMode & 2) {
return 0;
}
else if (_fastMode & 1) {
return 10;
}
else {
delayCount += _systemVars.gameSpeed;
int needDelay = delayCount - (int)_system->getMillis();
if ((needDelay < 0) || (needDelay > _systemVars.gameSpeed)) {
needDelay = 0;
delayCount = _system->getMillis();
}
return needDelay;
}
}
In main loop I’ve left only delay (calculateDelay(delayCount)) string. I don’t actually know, why delayCount uses in pause state, so I did’t touch it.
Next code I want to extract from main loop is
if (shouldPerformAutoSave(_lastSaveTime)) {
if (_skyControl->loadSaveAllowed()) {
_lastSaveTime = _system->getMillis();
_skyControl->doAutoSave();
} else
_lastSaveTime += 30 * 1000; // try again in 30 secs
}
It’s absolutely independent and we may extract it into separate method
void SkyEngine::performAutosave() {
if (shouldPerformAutoSave(_lastSaveTime)) {
if (_skyControl->loadSaveAllowed()) {
_lastSaveTime = _system->getMillis();
_skyControl->doAutoSave();
} else
_lastSaveTime += 30 * 1000; // try again in 30 secs
}
}
At the same time i’ve noticed that acutosaving doesn’t perform in pause state (bug?)
For the same reasons, I’ve extracted this code
if (_debugger->showGrid()) {
uint8 *grid = _skyLogic->_skyGrid->giveGrid(Logic::_scriptVariables[SCREEN]);
if (grid) {
_skyScreen->showGrid(grid);
_skyScreen->forceRefresh();
}
}
to separate method
void SkyEngine::showGrid() {
if (_debugger->showGrid()) {
uint8 *grid = _skyLogic->_skyGrid->giveGrid(Logic::_scriptVariables[SCREEN]);
if (grid) {
_skyScreen->showGrid(grid);
_skyScreen->forceRefresh();
}
}
}
in result of all these actions I got clean and readable main loop.
while (!shouldQuit()) {
if (_systemVars.paused) {
_system->updateScreen();
delay(50);
handleKey();
delayCount = _system->getMillis();
}
else {
_debugger->onFrame();
performAutosave();
_skySound->checkFxQueue();
_skyMouse->mouseEngine();
handleKey();
_skyLogic->engine();
_skyScreen->processSequence();
_skyScreen->recreate();
_skyScreen->spriteEngine();
showGrid();
_skyScreen->flip();
delay(calculateDelay(delayCount));
}
}
But it’s not the end. I still can make few improvements here. Each state call delay and handleKey functions. I think, I can move them out of condition. But to do this, I need to change calculateDelay function and add those strings to it:
if (_systemVars.paused) {
delayCount = _system->getMillis();
return 50;
}
Paused state calls _system->updateScreen() method. After few ticks function delay calls the same method. I not sure, but it seems like we can remove this call from paused state. Aad we get situation where engine doesnt perform any actions in paused state. I’ve changed condition and main loop got following look:
while (!shouldQuit()) {
if (!_systemVars.paused) {
_debugger->onFrame();
_skySound->checkFxQueue();
_skyMouse->mouseEngine();
_skyLogic->engine();
_skyScreen->processSequence();
_skyScreen->recreate();
_skyScreen->spriteEngine();
showGrid();
_skyScreen->flip();
}
performAutosave();
delay(calculateDelay(delayCount));
handleKey();
}
In my opinion, it much more readable and clear than it was. We can move forward and make it FSM-like and remove delay function and one more loop, but I don’t think we realy need to do this.
Now, let’s look to the delay() function.
void SkyEngine::delay(int32 amount) {
Common::Event event;
uint32 start = _system->getMillis();
_keyPressed.reset();
if (amount < 0) amount = 0; do { while (_eventMan->pollEvent(event)) {
switch (event.type) {
case Common::EVENT_KEYDOWN:
_keyPressed = event.kbd;
break;
case Common::EVENT_MOUSEMOVE:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
break;
case Common::EVENT_LBUTTONDOWN:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
_skyMouse->buttonPressed(2);
break;
case Common::EVENT_RBUTTONDOWN:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
_skyMouse->buttonPressed(1);
break;
default:
break;
}
}
_system->updateScreen();
if (amount > 0)
_system->delayMillis((amount > 10) ? 10 : amount);
} while (_system->getMillis() < start + amount);
}
There are 2 loops in delay() function. They make logicaly unrelated to each other actions. Outer loop makes delay, inner loop processes events. I think we need extract events processing to separate function (processEvents). This will make code more redable and willn’t change logic. Delay funtion became more understandable, cause programmer don’t need to filter event processing by his mind, he’s just seeing “processEvents” word and understands what’s going on. So, after extraction there are 2 functions:
void SkyEngine::delay(int32 amount) {
uint32 start = _system->getMillis();
_keyPressed.reset();
if (amount < 0) amount = 0; do { processEvents(); _system->updateScreen();
if (amount > 0)
_system->delayMillis((amount > 10) ? 10 : amount);
} while (_system->getMillis() < start + amount); } void SkyEngine::processEvents() { Common::Event event; while (_eventMan->pollEvent(event)) {
switch (event.type) {
case Common::EVENT_KEYDOWN:
_keyPressed = event.kbd;
break;
case Common::EVENT_MOUSEMOVE:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
break;
case Common::EVENT_LBUTTONDOWN:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
_skyMouse->buttonPressed(2);
break;
case Common::EVENT_RBUTTONDOWN:
if (!(_systemVars.systemFlags & SF_MOUSE_LOCKED))
_skyMouse->mouseMoved(event.mouse.x, event.mouse.y);
_skyMouse->buttonPressed(1);
break;
default:
break;
}
}
}
It isn’t correct for this miment, so I’ll put it under cut
Here you can see my vision of EventRecorder I’ve implemented. I’ve summoned all my mad msPaint skills and hopeing that it looking as well clear as funny. Let me describe how it works:
I want to take part in summvm development as a part of the Google Summer of Code Event. Here I’ll tell about work done, about my impressions from this project and about other things related to this.
It was highly recommended to make bugfix or path of existing code for participation in the project. But I have some problems with them. After trying about ten bugs from bugtracker, I can’t reproduce any of them. That’s why, I’m going to start working on the idea I want to implement during this project.