-
Notifications
You must be signed in to change notification settings - Fork 55
Add coroutines await() to signals #660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1b3ce84 to
f1981cc
Compare
f1981cc to
ada89ae
Compare
5a448ec to
4f5785c
Compare
ada89ae to
d5f38e7
Compare
6cc3d6e to
5db9b2a
Compare
018d284 to
587e9f3
Compare
587e9f3 to
21283d1
Compare
f213992 to
f017f4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you check that none of the IDE checks are requiring the signals to be in the signal package?
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Show resolved
Hide resolved
kt/godot-coroutine-library/src/main/kotlin/godot/coroutines/GodotCoroutine.kt
Show resolved
Hide resolved
kt/plugins/godot-gradle-plugin/src/main/kotlin/godot/gradle/GodotPlugin.kt
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/extensions/TypedExtensions.kt
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Outdated
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Outdated
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Outdated
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Outdated
Show resolved
Hide resolved
kt/api-generator/src/main/kotlin/godot/codegen/services/impl/AwaitGenerationService.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What i meant with tests which switch coroutine context is that you await a signal in a test using withContext(Dispatcher.SomeOtherdispatcherThanYouStartedWith) { signal.await() } But that can be added later as well.
Other than that I'm happy :-) Good first implementation with which we can start with and have a look how it feels.
kotlinx.coroutines)await()functions to all signals that will suspend the coroutines until the signal is emitted.await()can return the signal's arguments, directly (when 1), as a tuple (when 2 and 3), and as list (4+)I also changed the package of Signal and Callable (I needed them for the await implementation so I took the opportunity).
So far everything has been in the same
godot.corepackage except for those 2. Maybe it's an error and we should split everything into smaller packages but right now I wanted to focus on more consistency.Note that the await() functions are in the
godot.coroutinespackage as it's not considered the same library.