Add higher-level finishCallback closure #46
Conversation
|
Not sure how to fix the Lint issues. Also, I re-pushed the branch, after rebasing locally. Don't like seeing merge commits in topic branches. |
Also fixes a bug in .load(path:) that was returning false when file was successfully loaded.
pcbeard
force-pushed
the
FinishCallbackClosure
branch
from
73eb6c9 to
216899b
Compare
| @discardableResult public func load(path: StaticString) -> Bool { | ||
| fileplayer.loadIntoPlayer(pointer, path.utf8Start) == 0 | ||
| fileplayer.loadIntoPlayer(pointer, path.utf8Start) == 1 | ||
| } | ||
|
|
||
| /// Prepares player to stream the file at path. Returns `true` if the file exists, otherwise `false`. | ||
| @discardableResult public func load(path: UnsafePointer<CChar>) -> Bool { | ||
| fileplayer.loadIntoPlayer(pointer, path) == 0 | ||
| fileplayer.loadIntoPlayer(pointer, path) == 1 | ||
| } |
There was a problem hiding this comment.
I think it would be nice to eventually make these throw an error if the file doesn't exist instead of returning false, but this is a good fix for now
| public var finishCallback: (() -> Void)? { | ||
| get { | ||
| _callbackData?.pointee.callback | ||
| } | ||
| set { | ||
| if let callbackData = _callbackData { | ||
| callbackData.pointee.callback = newValue | ||
| } else if let newValue { | ||
| let callbackData: UnsafeMutablePointer<CallbackData> = .allocate(capacity: 1) | ||
| callbackData.initialize(to: .init(callback: newValue)) | ||
| _callbackData = callbackData | ||
| setFinishCallback( | ||
| callback: { sourceSource, userdata in | ||
| if let callback = userdata?.assumingMemoryBound(to: CallbackData.self).pointee.callback { | ||
| callback() | ||
| } | ||
| }, | ||
| soundUserdata: callbackData | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems like a nice approach for this. Alternatives would be to mark FilePlayer as open, set the userdata to a pointer to the file player, and have it call an open didFinish() method that people can override (similar to what I'm doing in #32). Another option might be to allow setting a delegate on the FilePlayer, which would align with iOS APIs.
Don't really have much of a preference here, just thinking out loud.
There was a problem hiding this comment.
Another approach might pass the FilePlayer as a parameter to the closure. This may be preferable, to avoid creating cycles, since weak is unavailable in embedded Swift (which is a shame). The only tricky bit is that the FilePlayer pointer would have to be stored as well in the callback, as you mentioned above. And sub-classing seems less flexible to me.
There was a problem hiding this comment.
One other tiny change might be to reorder the parameters of setFinishCallback() so a trailing closure can be used.
Formatting is done using |
Also fixes a bug in .load(path:) that was returning false when file was successfully loaded.