Conversation
1. Callbacks set via setTrackEndListener are not triggered. 2. The setEndListener callback interface cannot retrieve the animation name through the trackEntry parameter.
| for (int i = 0; i < vecTrackEvents.size(); i++) { | ||
| auto& info = vecTrackEvents[i]; | ||
| onTrackEntryEvent(info.entry, info.eventType, info.event); | ||
| } |
There was a problem hiding this comment.
Optimize the code by checking whether the original vector is emtpy to avoid creating an empty vector and copying the empty vector.
if (animationEvents.size() > 0) {
// Cache animation events then call back to JS.
auto vecAnimationEvents = animationEvents;
animationEvents.clear();
for (int i = 0; i < vecAnimationEvents.size(); i++) {
auto& info = vecAnimationEvents[i];
onAnimationStateEvent(info.entry, info.eventType, info.event);
}
}
if (trackEvents.size() > 0) {
// Cache track events then call back to JS.
auto vecTrackEvents = trackEvents;
trackEvents.clear();
for (int i = 0; i < vecTrackEvents.size(); i++) {
auto& info = vecTrackEvents[i];
onTrackEntryEvent(info.entry, info.eventType, info.event);
}
}There was a problem hiding this comment.
Please notice that spine::Vector doesn't provide a operator= implementation, see
https://github.com/cocos/cocos-engine/blob/v3.8.7/native/cocos/editor-support/spine/3.8/spine/Vector.h#L231
So the copy assignment will use a default one which only assigns the buffer pointer and size. If so, we need to fix that for spine::Vector.
There was a problem hiding this comment.
Please notice that spine::Vector doesn't provide a operator= implementation
It seems to be fine that the assignment auto vecAnimationEvents = animationEvents will trigger the spine::Vector copy constructor.
There was a problem hiding this comment.
I wrote a test case that could reproduce the issue I said, but yes, it's not relevant to this PR.
{
spine::Vector<int> arr;
arr.add(1);
arr.add(2);
arr.add(3);
spine::Vector<int> arr2;
arr2 = arr; // --> call the default assignment operator
arr2[0] = 100;
printf("arr --------\n");
for (size_t i = 0; i < arr.size(); ++i) {
printf("%d, ", arr[i]);
}
printf("\n");
printf("arr2 --------\n");
for (size_t i = 0; i < arr2.size(); ++i) {
printf("%d, ", arr2[i]);
}
printf("\n");
printf("\n");
}The output is wrong:
arr --------
100, 2, 3,
arr2 --------
100, 2, 3,
There was a problem hiding this comment.
I sent a PR to spine runtime official repo ( EsotericSoftware/spine-runtimes#2828 ) to fix the copy assignment issue.
There was a problem hiding this comment.
Currently, the assignment type of 'operator =' is not used by spine's module
| * In the official implementation of Spine's AnimationState class, the animationCallback is invoked after the trackEntryCallback. | ||
| * After the AnimationState completes the EventType_Dispose callback, the TrackEntry will be reclaimed, so this event must be dispatched immediately. | ||
| */ | ||
| skeletonAnimation->dispatchEvents(); |
There was a problem hiding this comment.
Could this cause other issues if something is done in the dispose callback?
There was a problem hiding this comment.
I Think that EventType_End disposeTrackEntry as same as EventType_Dispose.
There was a problem hiding this comment.
EventType_end event is cached by cacheAnimationEvent, while EventType_Dispose is dispatched it will dispatch EventType_end firstly.
There was a problem hiding this comment.
Could this cause other issues if something is done in the
disposecallback?
trackEntry will be enter _trackEntryPool only after EventType_dispose is dispatched. So, it should not cause other issues.
Code Size Check Report
Interface Check ReportThis pull request does not change any public interfaces ! |
* Fix spine issues: 1. Callbacks set via setTrackEndListener are not triggered. 2. The setEndListener callback interface cannot retrieve the animation name through the trackEntry parameter. (cherry picked from commit 7ead7e4) # Conflicts: # native/cocos/editor-support/spine-wasm/spine-skeleton-instance.cpp resolved by theirs version # native/external-config.json
3 participants
Re: #
#18610
cocos/cocos-test-projects#920
cocos/cocos-engine-external#488
Changelog
Continuous Integration
This pull request:
Compatibility Check
This pull request: