{"id":20,"date":"2013-07-01T17:03:57","date_gmt":"2013-07-01T17:03:57","guid":{"rendered":"https:\/\/blogs.scummvm.org\/richiesams\/?p=20"},"modified":"2022-05-22T19:55:31","modified_gmt":"2022-05-22T19:55:31","slug":"improving-the-object-class-including-renaming-it-and-using-classes-for-resultactions","status":"publish","type":"post","link":"https:\/\/blogs.scummvm.org\/richiesams\/2013\/07\/01\/improving-the-object-class-including-renaming-it-and-using-classes-for-resultactions\/","title":{"rendered":"Improving the ‘Object’ class (including renaming it) and using classes for ResultActions"},"content":{"rendered":"

Last week, I posted about using an ‘Object’ class to encapsulate the variable-typed arguments for ResultActions. You guys posted some awesome feedback and I used it to improve the class. First, I renamed the class to ‘SingleValueContainer’ so users have a better sense of what it is. Second, following\u00a0Fuzzie’s advice<\/a>, I put all the values except for String, directly in the union. It’s the same or less memory cost and results in less heap allocations.<\/p>\n

union {\r\n    bool boolVal;\r\n    byte byteVal;\r\n    int16 int16Val;\r\n    uint16 uint16Val;\r\n    int32 int32Val;\r\n    uint32 uint32Val;\r\n    float floatVal;\r\n    double doubleVal;\r\n    char *stringVal;\r\n} _value;\r\n<\/pre>\n
You’ll notice that the stringVal isn’t actually a Common::String object, but rather a pointer to a char array. This saves a bit of memory at the cost of a couple strlen(), memcpy(), and String object assigment.<\/p>\n
SingleValueContainer::SingleValueContainer(Common::String value) : _objectType(BYTE) {\r\n    _value.stringVal = new char[value.size() + 1];\r\n    memcpy(_value.stringVal, value.c_str(), value.size() + 1);\r\n}\r\n<\/pre>\n
SingleValueContainer &SingleValueContainer::operator=(const Common::String &rhs) {\r\n    if (_objectType != STRING) {\r\n        _objectType = STRING;\r\n        _value.stringVal = new char[rhs.size() + 1];\r\n        memcpy(_value.stringVal, rhs.c_str(), rhs.size() + 1);\r\n\r\n        return *this;\r\n    }\r\n\r\n    uint32 length = strlen(_value.stringVal);\r\n    if (length <= rhs.size() + 1) {\r\n        memcpy(_value.stringVal, rhs.c_str(), rhs.size() + 1);\r\n    } else {\r\n        delete[] _value.stringVal;\r\n        _value.stringVal = new char[rhs.size() + 1];\r\n        memcpy(_value.stringVal, rhs.c_str(), rhs.size() + 1);\r\n    }\r\n\r\n    return *this;\r\n}\r\n<\/pre>\n
bool SingleValueContainer::getStringValue(Common::String *returnValue) const {\r\n    if (_objectType !=  STRING)\r\n        warning(\"'Object' is not storing a Common::String.\");\r\n\r\n    *returnValue = _value.stringVal;\r\n    return true;\r\n}\r\n<\/pre>\n
With those changes the class seems quite solid. (The full source can be found\u00a0here<\/a>\u00a0and\u00a0here<\/a>). However, after seeing\u00a0Zidane Sama’s comment<\/a>, I realized that there was a better way to tackle the problem than variant objects. Instead of trying to generalize the action types and arguments and storing them in structs, a better approach is to create a class for each action type with a common, “execute()” method that will be called by the scriptManager when the Criteria are met for an ResultAction.<\/p>\n
I first created an interface base class that all the different types would inherit from:<\/p>\n
class ResultAction {\r\npublic:\r\n    virtual ~ResultAction() {}\r\n    virtual bool execute(ZEngine *zEngine) = 0;\r\n};\r\n<\/pre>\n
Next, I created the individual classes for each type of ResultAction:<\/p>\n
class ActionAdd : public ResultAction {\r\npublic:\r\n    ActionAdd(Common::String line);\r\n    bool execute(ZEngine *zEngine);\r\n\r\nprivate:\r\n    uint32 _key;\r\n    byte _value;\r\n};\r\n<\/pre>\n
The individual classes parse out any arguments in their constructor and store them in member variables. In execute(), they execute the logic pertaining to their action. A pointer to ZEngine is passed in order to give the method access to all the necessary tools (modifying graphics, scriptManager states, sounds, etc.)<\/p>\n
class ResultAction {\r\nActionAdd::ActionAdd(Common::String line) {\r\n    sscanf(line.c_str(), \":add(%u,%hhu)\", &_key, &_value);\r\n}\r\n\r\nbool ActionAdd::execute(ZEngine *zEngine) {\r\n    zEngine->getScriptManager()->addToStateValue(_key, _value);\r\n    return true;\r\n}\r\n<\/pre>\n
Thus, in the script file parser I can just look for the action type and then pass create an action type, passing the constructor the whole line:<\/p>\n
while (!line.contains('}')) {\r\n    \/\/ Parse for the action type\r\n    if (line.matchString(\"*:add*\", true)) {\r\n        actionList.push_back(ActionAdd(line));\r\n    } else if (line.matchString(\"*:animplay*\", true)) {\r\n        actionList.push_back(ActionAnimPlay(line));\r\n    } else if (.....)\r\n         .\r\n         .\r\n         .\r\n}\r\n<\/pre>\n
While this means I have to create 20+ classes for all the different types of actions, I think this method nicely encapsulates and abstracts both the parsing and the action of the result.<\/p>\n
I’m a bit sad that I’m not going to be using the ‘SingleValueContainer’ class, but if nothing else, I learned quite a bit while creating it. Plus, I won’t be getting rid of it, so it might have a use somewhere else.<\/p>\n
This coming week I need to finish creating all the classes and then try to finish the rest of the engine skeleton. As always, feel free to comment \/ ask questions.<\/p>\n
-RichieSams<\/p>\n","protected":false},"excerpt":{"rendered":"
Last week, I posted about using an ‘Object’ class to encapsulate the variable-typed arguments for ResultActions. You guys posted some awesome feedback and I used it to improve the class. First, I renamed the class to ‘SingleValueContainer’ so users have a better sense of what it is. Second, following\u00a0Fuzzie’s advice, I put all the values […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"closed","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[1],"tags":[],"class_list":["post-20","post","type-post","status-publish","format-standard","hentry","category-uncategorized"],"_links":{"self":[{"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/posts\/20","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/comments?post=20"}],"version-history":[{"count":1,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/posts\/20\/revisions"}],"predecessor-version":[{"id":21,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/posts\/20\/revisions\/21"}],"wp:attachment":[{"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/media?parent=20"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/categories?post=20"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/blogs.scummvm.org\/richiesams\/wp-json\/wp\/v2\/tags?post=20"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}