Friday, July 13, 2012

Refactoring week


So, I'm still working on integration of save files into recording file. On last week I did alot of things to this. After code studying, I saw that every engine supporting states loading, have some kind of fucntion which generates file name by slot's number. Some engines have this function as is a class static method, others like public method, few engines have this function inlined to code of saves-related functions. In other words, we have same behavior and different interface. So after discussion with mentor we decided to unificete interface of save files name generation and make all engines compatible with this interface. Then we produced some requirements to name generation function:

1) It should be accessible without class instancing. Because MetaEngine use it for removeSaveState and querySaveMetaInfo functions.
2) It should be accessible inside of class methods (obviously)
3) It should be accessible outside of class using the pointer of abstract class Engine. It to make possible to use it from eventRecorder
4) Logic of name generation should be in one place. Because it's possible to get error relate to duplicated code. For instance, in current version of queen engine, function QueenEngine::makeGameStateName uses constant string "queen" and function QueenMetaEngine::removeSaveState uses variable target for name generation. This may cause a problems.
5) Preferably to have a fucntion which check number of saving's slot is special (for ecample used for autosave or recording file) and generate name without depending to engine implementation

As most engines generate file name by formula fileName = preffix + "." + pattern, where preffix is a name of engine and pattern is function from slot number (for example "%03d"). Firstly I thought that it would be a nice idea to make a universal method in Engine class. Parameters of this function would be preffix (usualy target name) and pattern to generate extension of save file. This method could look like Engine::getSaveName(prefix, pattern, slot). But some engine need different algorithm of names generation and each time we call this method we should pass to many params to it, which is not very comfortable and can be cause of errors.

Then I decided that it should be good to make and AbstractNameGenerator class with abstract generateSaveName method, and inherited it for each engine. You can see it on diagram:

I would be consistent to all requirements, but create class without state which contains only one method is definitely overengineerng.
So, we decided to make static method and wrap it by virtual function in each engine. And in every virtual function call checking for special slot.

And then fuzzie suggested to  wrap the SaveFileManager's openForLoading function, for the event recorder. And this idea looks very attractive, cause at first, it can be realized without changing of existing codem and at second it corresponds to the logic of other eventrecorder's systems. So at the moment I'm working on implementation this idea

No comments:

Post a Comment