Skip to content

Commit

Permalink
Refactor AnimationSet loading
Browse files Browse the repository at this point in the history 
Closes #962

Delegating to a copy constructor is potentially an unsafe operation as well as MSVC complaining that member variables were not initialized. All member variables are now initialized in the header by default.

I took the opportunity to refactor the loading code. The various `process` functions were private to the implementation file via an unnamed namespace as well as only used in one place they made great candidates for Immediately Invoked Initializing Lambdas which aren't accessible outside the function they are declared, reducing their scope even further.
  • Loading branch information
@cugone
cugone committed Jul 31, 2021
1 parent ef708c9 commit 0c2ef49
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 184 deletions.
334 changes: 153 additions & 181 deletions NAS2D/Resource/AnimationSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ namespace
{
return " (Row: " + std::to_string(row) + ")";
}

AnimationSet processXml(std::string filePath, ImageCache& imageCache);
std::map<std::string, std::string> processImageSheets(const std::string& basePath, const Xml::XmlElement* element, ImageCache& imageCache);
std::map<std::string, std::vector<AnimationSet::Frame>> processActions(const std::map<std::string, std::string>& imageSheetMap, const Xml::XmlElement* element, ImageCache& imageCache);
std::vector<AnimationSet::Frame> processFrames(const std::map<std::string, std::string>& imageSheetMap, const Xml::XmlElement* element, ImageCache& imageCache);
}


Expand All @@ -55,10 +50,36 @@ bool AnimationSet::Frame::isStopFrame() const
}


AnimationSet::AnimationSet(std::string fileName) : AnimationSet{processXml(std::move(fileName), animationImageCache)}
AnimationSet::AnimationSet(std::string fileName) :
mFileName{fileName}
{
try {
const auto& filesystem = Utility<Filesystem>::get();
const auto basePath = filesystem.parentPath(mFileName);

Xml::XmlDocument xmlDoc{};
xmlDoc.parse(filesystem.read(mFileName).c_str());

if(xmlDoc.error())
{
throw std::runtime_error("Sprite file has malformed XML: Row: " + std::to_string(xmlDoc.errorRow()) + " Column: " + std::to_string(xmlDoc.errorCol()) + " : " + xmlDoc.errorDesc());
}
loadFromXml(*xmlDoc.rootElement());
} catch(...) {
throw;
}
}

AnimationSet::AnimationSet(const Xml::XmlElement& element)
{
try {
loadFromXml(element);
}
catch(...)
{
throw;
}
}

AnimationSet::AnimationSet(std::string fileName, std::map<std::string, std::string> imageSheetMap, std::map<std::string, std::vector<Frame>> actions) :
mFileName{std::move(fileName)},
Expand All @@ -84,200 +105,151 @@ const std::vector<AnimationSet::Frame>& AnimationSet::frames(const std::string&
return mActions.at(actionName);
}


namespace
void AnimationSet::loadFromXml(const Xml::XmlElement& element)
{

/**
* Parses a Sprite XML Definition File.
*
* \param filePath File path of the sprite XML definition file.
*/
AnimationSet processXml(std::string filePath, ImageCache& imageCache)
try
{
try
if (const auto* xml_sprite = element.firstChildElement("sprite"); xml_sprite == nullptr)
{
auto& filesystem = Utility<Filesystem>::get();
const auto basePath = filesystem.parentPath(filePath);

Xml::XmlDocument xmlDoc;
xmlDoc.parse(filesystem.read(filePath).c_str());

if (xmlDoc.error())
{
throw std::runtime_error("Sprite file has malformed XML: Row: " + std::to_string(xmlDoc.errorRow()) + " Column: " + std::to_string(xmlDoc.errorCol()) + " : " + xmlDoc.errorDesc());
}

// Find the Sprite node.
const auto* xmlRootElement = xmlDoc.firstChildElement("sprite");
if (!xmlRootElement)
{
throw std::runtime_error("Sprite file does not contain required <sprite> tag");
}

// Get the Sprite version.
const auto version = xmlRootElement->attribute("version");
if (version.empty())
throw std::runtime_error("Sprite file does not contain required <sprite> tag");
}
else
{
if(const auto version = element.attribute("version"); version.empty())
{
throw std::runtime_error("Sprite file's root element does not specify a version");
}
if (version != SPRITE_VERSION)
} else
{
throw std::runtime_error("Sprite version mismatch. Expected: " + std::string{SPRITE_VERSION} + " Actual: " + versionString());
if(version != SPRITE_VERSION)
{
throw std::runtime_error("Sprite version mismatch. Expected: " + std::string{SPRITE_VERSION} + " Actual: " + versionString());
}
}

// Note:
// Here instead of going through each element and calling a processing function to handle
// it, we just iterate through all nodes to find sprite sheets. This allows us to define
// image sheets anywhere in the sprite file.
auto imageSheetMap = processImageSheets(basePath, xmlRootElement, imageCache);
auto actions = processActions(imageSheetMap, xmlRootElement, imageCache);
return {std::move(filePath), std::move(imageSheetMap), std::move(actions)};
}
catch(const std::runtime_error& error)
mImageSheetMap = [&]() //IIIL
{
throw std::runtime_error("Error parsing Sprite file: " + filePath + "\nError: " + error.what());
}
}


/**
* Iterates through all elements of a Sprite XML definition looking
* for 'imagesheet' elements and processes them.
*
* \note Since 'imagesheet' elements are processed before any other
* element in a sprite definition, these elements can appear
* anywhere in a Sprite XML definition.
*/
std::map<std::string, std::string> processImageSheets(const std::string& basePath, const Xml::XmlElement* element, ImageCache& imageCache)
{
std::map<std::string, std::string> imageSheetMap;

for (const auto* node = element->firstChildElement("imagesheet"); node; node = node->nextSiblingElement("imagesheet"))
{
const auto dictionary = attributesToDictionary(*node);
const auto id = dictionary.get("id");
const auto src = dictionary.get("src");

if (id.empty())
{
throw std::runtime_error("Sprite imagesheet definition has `id` of length zero: " + endTag(node->row()));
}

if (src.empty())
{
throw std::runtime_error("Sprite imagesheet definition has `src` of length zero: " + endTag(node->row()));
}
std::map<std::string, std::string> imageSheetMap;

if (imageSheetMap.find(id) != imageSheetMap.end())
for (const auto* node = element.firstChildElement("imagesheet"); node != nullptr; node = node->nextSiblingElement("imagesheet"))
{
throw std::runtime_error("Sprite image sheet redefinition: id: '" + id + "' " + endTag(node->row()));
}

const auto imagePath = basePath + src;
imageSheetMap.try_emplace(id, imagePath);
imageCache.load(imagePath);
}

return imageSheetMap;
}
const auto dictionary = attributesToDictionary(*node);
const auto id = dictionary.get("id");
const auto src = dictionary.get("src");

if (id.empty())
{
throw std::runtime_error("Sprite imagesheet definition has `id` of length zero: " + endTag(node->row()));
}

/**
* Iterates through all elements of a Sprite XML definition looking
* for 'action' elements and processes them.
*/
std::map<std::string, std::vector<AnimationSet::Frame>> processActions(const std::map<std::string, std::string>& imageSheetMap, const Xml::XmlElement* element, ImageCache& imageCache)
{
std::map<std::string, std::vector<AnimationSet::Frame>> actions;

for (const auto* action = element->firstChildElement("action"); action; action = action->nextSiblingElement("action"))
{
const auto dictionary = attributesToDictionary(*action);
const auto actionName = dictionary.get("name");
if (src.empty())
{
throw std::runtime_error("Sprite imagesheet definition has `src` of length zero: " + endTag(node->row()));
}

if (actionName.empty())
{
throw std::runtime_error("Sprite Action definition has 'name' of length zero: " + endTag(action->row()));
}
if (actions.find(actionName) != actions.end())
{
throw std::runtime_error("Sprite Action redefinition: '" + actionName + "' " + endTag(action->row()));
}
if (imageSheetMap.find(id) != imageSheetMap.end())
{
throw std::runtime_error("Sprite image sheet redefinition: id: '" + id + "' " + endTag(node->row()));
}

actions[actionName] = processFrames(imageSheetMap, action, imageCache);
const auto& filesystem = Utility<Filesystem>::get();
const auto basePath = filesystem.parentPath(mFileName);

if (actions[actionName].empty())
{
throw std::runtime_error("Sprite Action contains no valid frames: " + actionName);
const auto path = basePath + src;
imageSheetMap.try_emplace(id, path);
animationImageCache.load(path);
}
}
return imageSheetMap;
}(); //IIIL

return actions;
mActions = [&]() //IIIL
{
std::map<std::string, std::vector<AnimationSet::Frame>> actions;

for (const auto* action = element.firstChildElement("action"); action != nullptr; action = action->nextSiblingElement("action"))
{
const auto dictionary = attributesToDictionary(*action);
const auto actionName = dictionary.get("name");

if (actionName.empty())
{
throw std::runtime_error("Sprite Action definition has 'name' of length zero: " + endTag(action->row()));
}
if (actions.find(actionName) != actions.end())
{
throw std::runtime_error("Sprite Action redefinition: '" + actionName + "' " + endTag(action->row()));
}

actions[actionName] = [&]() //IIIL
{
std::vector<AnimationSet::Frame> frameList;

for (const auto* frame = element.firstChildElement("frame"); frame; frame = frame->nextSiblingElement("frame"))
{
const auto currentRow = frame->row();

const auto dictionary = attributesToDictionary(*frame);
reportMissingOrUnexpected(dictionary.keys(), {"sheetid", "x", "y", "width", "height", "anchorx", "anchory"}, {"delay"});

const auto sheetId = dictionary.get("sheetid");
const auto delay = dictionary.get<unsigned int>("delay", 0);
const auto x = dictionary.get<int>("x");
const auto y = dictionary.get<int>("y");
const auto width = dictionary.get<int>("width");
const auto height = dictionary.get<int>("height");
const auto anchorx = dictionary.get<int>("anchorx");
const auto anchory = dictionary.get<int>("anchory");

if (sheetId.empty())
{
throw std::runtime_error("Sprite Frame definition has 'sheetid' of length zero: " + endTag(currentRow));
}
if (const auto iterator = mImageSheetMap.find(sheetId); iterator != mImageSheetMap.end())
{
const auto& image = animationImageCache.load(iterator->second);
// X-Coordinate
if (x < 0 || x > image.size().x)
{
throw std::runtime_error("Sprite frame attribute 'x' is out of bounds: " + endTag(currentRow));
}
// Y-Coordinate
if (y < 0 || y > image.size().y)
{
throw std::runtime_error("Sprite frame attribute 'y' is out of bounds: " + endTag(currentRow));
}
// Width
if (width <= 0 || width > image.size().x - x)
{
throw std::runtime_error("Sprite frame attribute 'width' is out of bounds: " + endTag(currentRow));
}
// Height
if (height <= 0 || height > image.size().y - y)
{
throw std::runtime_error("Sprite frame attribute 'height' is out of bounds: " + endTag(currentRow));
}

const auto bounds = Rectangle<int>::Create(Point<int>{x, y}, Vector{width, height});
const auto anchorOffset = Vector{anchorx, anchory};
frameList.emplace_back(AnimationSet::Frame{image, bounds, anchorOffset, delay});
}
else
{
throw std::runtime_error("Sprite Frame definition references undefined imagesheet: '" + sheetId + "' " + endTag(currentRow));
}
}

return frameList;
}(); //IIIL

if (actions[actionName].empty())
{
throw std::runtime_error("Sprite Action contains no valid frames: " + actionName);
}
}
return actions;
}(); //IIIL
}


/**
* Parses through all <frame> tags within an <action> tag in a Sprite Definition.
*/
std::vector<AnimationSet::Frame> processFrames(const std::map<std::string, std::string>& imageSheetMap, const Xml::XmlElement* element, ImageCache& imageCache)
catch (...)
{
std::vector<AnimationSet::Frame> frameList;

for (const auto* frame = element->firstChildElement("frame"); frame; frame = frame->nextSiblingElement("frame"))
{
int currentRow = frame->row();

const auto dictionary = attributesToDictionary(*frame);
reportMissingOrUnexpected(dictionary.keys(), {"sheetid", "x", "y", "width", "height", "anchorx", "anchory"}, {"delay"});

const auto sheetId = dictionary.get("sheetid");
const auto delay = dictionary.get<unsigned int>("delay", 0);
const auto x = dictionary.get<int>("x");
const auto y = dictionary.get<int>("y");
const auto width = dictionary.get<int>("width");
const auto height = dictionary.get<int>("height");
const auto anchorx = dictionary.get<int>("anchorx");
const auto anchory = dictionary.get<int>("anchory");

if (sheetId.empty())
{
throw std::runtime_error("Sprite Frame definition has 'sheetid' of length zero: " + endTag(currentRow));
}
const auto iterator = imageSheetMap.find(sheetId);
if (iterator == imageSheetMap.end())
{
throw std::runtime_error("Sprite Frame definition references undefined imagesheet: '" + sheetId + "' " + endTag(currentRow));
}

const auto& image = imageCache.load(iterator->second);
// X-Coordinate
if (x < 0 || x > image.size().x)
{
throw std::runtime_error("Sprite frame attribute 'x' is out of bounds: " + endTag(currentRow));
}
// Y-Coordinate
if (y < 0 || y > image.size().y)
{
throw std::runtime_error("Sprite frame attribute 'y' is out of bounds: " + endTag(currentRow));
}
// Width
if (width <= 0 || width > image.size().x - x)
{
throw std::runtime_error("Sprite frame attribute 'width' is out of bounds: " + endTag(currentRow));
}
// Height
if (height <= 0 || height > image.size().y - y)
{
throw std::runtime_error("Sprite frame attribute 'height' is out of bounds: " + endTag(currentRow));
}

const auto bounds = Rectangle<int>::Create(Point<int>{x, y}, Vector{width, height});
const auto anchorOffset = Vector{anchorx, anchory};
frameList.push_back(AnimationSet::Frame{image, bounds, anchorOffset, delay});
}

return frameList;
throw;
}

}
Loading

0 comments on commit 0c2ef49

Please sign in to comment.