From a8ca019d184e71f7a67fa7392b0ee59d74cc33bf Mon Sep 17 00:00:00 2001 From: Dmitriy Date: Fri, 21 Apr 2023 20:32:03 +0300 Subject: [PATCH] Handle skeleton loading problems without crashes This pull request improve the loading of binary skeleton so that there is no crash in case of attachment problems. Essentially, it will have the same behavior as for JSON skeleton, just returning NULL, and you can print pSkeleton->error to the output for the debug purpose. The functions spSkeletonBinary_create() and spSkeletonJson_create() both use spAtlasAttachmentLoader, which creates attachments in its _spAtlasAttachmentLoader_createAttachment(). This function can return NULL in case of any problems with the atlas regions also setting internal error1,error2 to "Region not found: ", region_path In case of SkeletonJson.c, there is a check for attachment != NULL after calling spAttachmentLoader_createAttachment(). However, unfortunately, there is no such check for SkeletonBinary.c, and if the region is not found, there is simply a crash deep inside the spine lib. I fixed it like this Currently, all calls to spAttachmentLoader_createAttachment() in SkeletonBinary.c are located inside the spSkeletonBinary_readAttachment() function, which is convenient. It is possible to check for the validity of the attachment after each _createAttachment() call. We can use if (!attachment) return NULL; In fact, the problem with the attachment can only be in three cases inside spSkeletonBinary_readAttachment() case SP_ATTACHMENT_REGION: case SP_ATTACHMENT_MESH: case SP_ATTACHMENT_LINKED_MESH: But for the sake of consistency, it may be worth checking the other four cases: SP_ATTACHMENT_BOUNDING_BOX: SP_ATTACHMENT_PATH: SP_ATTACHMENT_POINT: SP_ATTACHMENT_CLIPPING: (I do attachment check here, but maybe we can just "trust" that the attachment is always valid in this cases). Now that spSkeletonBinary_readAttachment() can return NULL instead of crashing, so go on and fix the next function - spSkeletonBinary_readSkin() Here, we also check the attachment for validity in this code 'for (i = 0; i < slotCount; ++i)'. Unfortunately, we cannot use 'continue' inside the loop here, as it is done in SkeletonJson.c, as we cannot move 'input->cursor' to the beginning of the data for the next slot. Here, we can only exit using return NULL, which generally suits us And finally, in the function spSkeletonBinary_readSkeletonData(), there are only two calls to spSkeletonBinary_readSkin() - one for the default skin and another inside the loop for other skins. Here, you can apply a check 'if(self->attachmentLoader->error1)', and in case of problems, clear the data by spSkeletonData_dispose() and exit with return NULL; similarly to how it is done below for errors "Skin not found: " and "Parent mesh not found: ". Thus, there will be no crash inside spine lib in case of attachment problems. Instead, NULL will be returned when calling spSkeletonBinary_readSkeletonData(), which the game engine can recognize and output the error message from pSkeleton->error in the output. This will help understand why my wonderful spine animation is not loading. --- spine-c/spine-c/src/spine/SkeletonBinary.c | 32 ++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/spine-c/spine-c/src/spine/SkeletonBinary.c b/spine-c/spine-c/src/spine/SkeletonBinary.c index 19fff284ae..9bf744ee15 100644 --- a/spine-c/spine-c/src/spine/SkeletonBinary.c +++ b/spine-c/spine-c/src/spine/SkeletonBinary.c @@ -1031,6 +1031,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput { spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, path, sequence); + if (!attachment) + return NULL; spRegionAttachment *region = SUB_CAST(spRegionAttachment, attachment); region->path = path; region->rotation = rotation; @@ -1051,6 +1053,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput int vertexCount = readVarint(input, 1); spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, 0, NULL); + if (!attachment) + return NULL; spVertexAttachment *vertexAttachment = SUB_CAST(spVertexAttachment, attachment); _readVertices(self, input, &vertexAttachment->bonesCount, &vertexAttachment->bones, &vertexAttachment->verticesCount, &vertexAttachment->vertices, @@ -1102,6 +1106,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput { spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, path, sequence); + if (!attachment) + return NULL; spMeshAttachment *mesh = SUB_CAST(spMeshAttachment, attachment); mesh->path = path; spColor_setFromColor(&mesh->color, &color); @@ -1151,6 +1157,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput { spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, path, sequence); + if (!attachment) + return NULL; spMeshAttachment *mesh = SUB_CAST(spMeshAttachment, attachment); mesh->path = path; spColor_setFromColor(&mesh->color, &color); @@ -1164,6 +1172,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput case SP_ATTACHMENT_PATH: { spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, 0, NULL); + if (!attachment) + return NULL; spPathAttachment *path = SUB_CAST(spPathAttachment, attachment); spVertexAttachment *vertexAttachment = SUPER(path); int vertexCount = 0; @@ -1187,6 +1197,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput case SP_ATTACHMENT_POINT: { spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, 0, NULL); + if (!attachment) + return NULL; spPointAttachment *point = SUB_CAST(spPointAttachment, attachment); point->rotation = readFloat(input); point->x = readFloat(input) * self->scale; @@ -1203,6 +1215,8 @@ spAttachment *spSkeletonBinary_readAttachment(spSkeletonBinary *self, _dataInput int vertexCount = readVarint(input, 1); spAttachment *attachment = spAttachmentLoader_createAttachment(self->attachmentLoader, skin, type, name, 0, NULL); + if (!attachment) + return NULL; spClippingAttachment *clip = SUB_CAST(spClippingAttachment, attachment); spVertexAttachment *vertexAttachment = SUPER(clip); _readVertices(self, input, &vertexAttachment->bonesCount, &vertexAttachment->bones, @@ -1253,7 +1267,9 @@ spSkin *spSkeletonBinary_readSkin(spSkeletonBinary *self, _dataInput *input, int const char *name = readStringRef(input, skeletonData); spAttachment *attachment = spSkeletonBinary_readAttachment(self, input, skin, slotIndex, name, skeletonData, nonessential); - if (attachment) spSkin_setAttachment(skin, slotIndex, name, attachment); + if (!attachment) + return NULL; + spSkin_setAttachment(skin, slotIndex, name, attachment); } } return skin; @@ -1508,6 +1524,11 @@ spSkeletonData *spSkeletonBinary_readSkeletonData(spSkeletonBinary *self, const /* Default skin. */ skeletonData->defaultSkin = spSkeletonBinary_readSkin(self, input, -1, skeletonData, nonessential); + if (self->attachmentLoader->error1) { + spSkeletonData_dispose(skeletonData); + _spSkeletonBinary_setError(self, self->attachmentLoader->error1, self->attachmentLoader->error2); + return NULL; + } skeletonData->skinsCount = readVarint(input, 1); if (skeletonData->defaultSkin) @@ -1520,7 +1541,13 @@ spSkeletonData *spSkeletonBinary_readSkeletonData(spSkeletonBinary *self, const /* Skins. */ for (i = skeletonData->defaultSkin ? 1 : 0; i < skeletonData->skinsCount; ++i) { - skeletonData->skins[i] = spSkeletonBinary_readSkin(self, input, 0, skeletonData, nonessential); + spSkin *skin = spSkeletonBinary_readSkin(self, input, 0, skeletonData, nonessential); + if (self->attachmentLoader->error1) { + spSkeletonData_dispose(skeletonData); + _spSkeletonBinary_setError(self, self->attachmentLoader->error1, self->attachmentLoader->error2); + return NULL; + } + skeletonData->skins[i] =skin; } /* Linked meshes. */ @@ -1575,6 +1602,7 @@ spSkeletonData *spSkeletonBinary_readSkeletonData(spSkeletonBinary *self, const if (!animation) { FREE(input); spSkeletonData_dispose(skeletonData); + _spSkeletonBinary_setError(self, "Animation corrupted: ", name); return NULL; } skeletonData->animations[i] = animation;