[spine_c] spSlot_setAttachment does not increment attachment refcount.

[brandon-reinhart]

There may be an internal reason I haven't found for this, but spSlot_setAttachment does not increment the internal refcount of the attachment assigned to the slot. This means it is possible to assign a custom attachment to a slot, dispose the custom attachment, and cause an access violation.

It seems to me that spSlot_setAttachment should dispose the attachment if it is replaced by null or another attachment and it should increment the refcount when the attachment is assigned.

By comparison, this is how spSkin_setAttachment works.

[brandon-reinhart]

ref: jabuwu/rusty_spine#19
ref: jabuwu/rusty_spine#20
ref: #2399

[badlogic]

Slots do not own attachments, skins do. That's why you see reference counting implemented for attachments only in spSkin_* and nowhere else, as it makes reasoning about life-times easier. A skin owns attachments, if you want the attachements gone, dispose the skin. Multiple skins can reference the same attachment, e.g. if you want to create a custom skin from attachments of two or more other skins. That's why there is reference counting.

That said, you can of course manually increment the ref count of an attachment anywhere, or simply never touch the ref count, and thereby manage it's life-time yourself.

Does this help your use case?

[brandon-reinhart]

I don't understand the reasoning. The api allows you to set an attachment on a slot in a way that will certainly crash. If this is unintended, the API should change.

Also, it seems fine - and works - to create a skeleton, export it without any attachments, and then build those attachments at runtime paperdoll style. Why is a skin needed to intermediate this?

[brandon-reinhart]

The runtime documentation even points out this crashy path and doesn't indicate that a skin should be required:

http://esotericsoftware.com/spine-using-runtimes/

At any given time, a slot can have a single attachment or no attachment. The attachment for a slot can be changed by calling setAttachment on the Skeleton or a Slot. The attachment will stay until changed again.

Skeleton skeleton = ...
// Finds the slot by name, finds the attachment by name in the skeleton's skin or default
// skin and sets it on the slot.
skeleton.setAttachment("slotName", "attachmentName");
// Attachments can be gotten from a skin or created manually (though this is advanced).
Attachment attachment = ...
// An attachment can be set directly on a slot.
skeleton.findSlot("slotName").setAttachment(attachment);

Attachments may be changed in other ways. Calling Skeleton setSlotsToSetupPose will change attachments. An animation may have keyframes that change attachments. Calling Skeleton setSkin may change attachments (see [Skin changes](http://esotericsoftware.com/spine-using-runtimes/#Skin-changes)).

[badlogic]

I think you misunderstood me. A skin is not needed to manage attachments you yourself allocate and which do not come from the skeleton data (.json or .skel files). In that case, the ref count is also not required: it's your responsibility to manage the life time of the attachment. If you set a disposed attachment on a slot, that's on you.

The ref count in attachments ONLY exists to enable this use case combining multiple skins (or a subset of their attachments) into a new skin:
https://github.com/EsotericSoftware/spine-runtimes/blob/4.1/spine-sfml/cpp/example/main.cpp#L576-L587

Here, the ref count makes it so that users of the API do not have to think about the life-time of attachments shared by multiple skins, as examplified in the code.

The ref count has no use other than enabling this one use case without introducing additional manual memory management on the user's end.

If you create attachments yourself in a language with manual memory management, it is on you to manage their life-times. See also the spine-c documentation: https://en.esotericsoftware.com/spine-c#Memory-management

As for the "crashy path": 1. you are looking at the general API documentation which does not talk about memory management in languages like C or C++. Also, it is absolutely unclear to me how the code you quoted is a "crashy path". This code will work in C as well, unless you set a disposed attachment on a slot. Which again, is your responsibility in terms of life-time management.

[brandon-reinhart]

AH. I see. I was operating under a misapprehension. My mistake.

[badlogic]

Sorry, I should have gone into more detail in my initial comment. Been a long day :)