Fix sequence hold handle

[amenonegames]

Issue1 : early return

The OnComplete method of the Sequence contained an early return statement: if (!handle.IsActive()) return;. However, since OnComplete is always called after the State transitions to Complete, the early return clause was always executed.

Issue2 : Preserved handle

Additionally, when adding a handle to the sequence, the Preserve flag was automatically set.
As a result, handles remained in memory even after the sequence execution completed.

Resolution1 : early return

I added an IsValid method that checks only whether the handle's version is appropriate and whether the handle exists in storage, without referencing the state.
The early return clause in Sequence.OnComplete was replaced with this IsValid check.

Resolution2 : Preserved handle

Additionally, upon the sequence's overall completion, we modified the behavior to cancel all handles within the sequence and discard preserved handles.

Verification

Verified execution using a combination of Join/Append/AppendInterval.
The debug screen results at the time of Motion completion are as follows.

before	after

※ I tested it in following repository.
https://github.com/amenonegames/LitMotionSequenceTest

Related Issue

#209

[amenonegames]

The Join operation seems to work fine, but an error occurred during the Append operation. I will conduct a follow-up investigation.
Fixed this.

[vietdungdev]

I will merge it into my fork <3

[nuskey8]

Merged the PR. Thank you!!!

[vietdungdev]

After careful consideration, I found that only one line of code modification is needed in the MotionSequenceSource.cs file.
Adding the check IsInvalid function is not necessary.

[amenonegames]

Thank you for vietdungdev's investigation!
Actually, I have considered the suggested fix before create pr. While vietdungdev may already be aware, I'll share the concerns I considered for reference.

The original if (!handle.IsActive()) in this section contained two function:

• function check if is the Version correct and the Handle stored in Storage
• function check if is the Handle's State either Running or Preserved

Therefore, I believe there are two possible interpretations for the original intent of placing if (!handle.IsActive()):

1. Intended to verify the Version and Storage storage were appropriate before executing
2. Intended to execute only if the State was not Running, but this was a typo

I chose interpretation 1 and opted to retain the IsValid check.
The reason is that if a method were executed on an invalid Handle, it could potentially cause an Exception.

This decision was simply based on my incomplete understanding of the library as a whole, opting for a safer approach that seemed to minimize review burden.
If vietdungdev's suggestion is appropriate, I would appreciate it if you could consider it !

[vietdungdev]

@amenonegames
I think it's a typo because it makes more sense if the result is reversed.
You can see the OnComplete call logic below:

In UpdateRunner.cs

if (status is MotionStatus.Completed && state.WasStatusChanged)
{
	managedData.InvokeOnComplete();
}

In MotionManager.cs

public bool IsActive(MotionHandle handle)
{
	ref var state = ref unmanagedDataArray[slot.DenseIndex].Core.State;
	return state.Status is MotionStatus.Scheduled or MotionStatus.Delayed or MotionStatus.Playing ||
										(state.Status is MotionStatus.Completed && state.IsPreserved);
}

This means that in the OnComplete function, if the handle is not IsPreserved, IsActive will always be false because state.Status is now MotionStatus.Completed.
If the handle is complete, there is no reason to prevent memory release.

By modifying the function as below, the new logic will be to check handle.IsActive() to determine if this handle is IsPreserved.
If it is not IsPreserved, it means IsActive == false, the handle is completed and the memory for the sequence and child handles in the item.Handle needs to be released.
If it is IsPreserved, then IsActive == true, meaning it needs to be kept for the user to run again later; the user will manually call Cancel or Complete.

void OnComplete()
{
	if (handle.IsActive()) return;
	foreach (var item in Items)
	{
		MotionManager.Cancel(item.Handle, false);
	} 

	Return(this); 
}

[vietdungdev]

@nuskey8
I see many useful pull requests for bug fixes, but you don't seem to have considered them.
However, this particular pull request was merged quite quickly.
Is there a special reason for that?

[vietdungdev]

If this commit is merged, it will automatically release the sequence's handles even if handle.Preserved() was called previously, which is clearly contrary to the original design.