Skip to content
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

Add notification ordering to Kotlin scripts. #495

Closed
CedNaru opened this issue Sep 15, 2023 · 2 comments
Closed

Add notification ordering to Kotlin scripts. #495

CedNaru opened this issue Sep 15, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@CedNaru
Copy link
Member

CedNaru commented Sep 15, 2023

Godot notification function is a special case in the Object API. At first glance it looks like a regular virtual function but behind the hood it's using some macro black magic to call all the _notification implementations in the class hierarchy.
It also have the option to make calls in reverse order (so from children's notifications to parent's notifications).
Previously, Scripts and Extensions were not part of the ordering but it changed in a recent PR (from an issue I posted myself :p):
godotengine/godot#78634

In that PR, the "reversed" parameter is added to the notification method for scripts and extensions. The expected behaviour is the same as native Godot classes, the hierarchy of scripts' _notification should be called in the correct order. We are not supposed to rely on the user making a super call everytime the previous notification is overridden.
We have to investigate if the current KotlinScript structure allows for that as we need to be able to access the parent script notification method as well.

If I remember correctly, we do things differently from the rest of Godot. We simply store all methods a script has, even the ones inherited from the parents if not overridden. Godot doesn't make any "copies" and relies on recursive calls to parents for everything, nut just notifications.

@CedNaru CedNaru changed the title add notification ordering to Kotlin scripts. Add notification ordering to Kotlin scripts. Sep 15, 2023
@CedNaru CedNaru added the enhancement New feature or request label Oct 3, 2023
@CedNaru CedNaru added this to the 0.8.0 milestone Oct 18, 2023
@CedNaru
Copy link
Member Author

CedNaru commented Nov 26, 2023

Proposal for the implementation:

@JvmInline
value class GodotNotification internal constructor(val block: Any.(Int) -> Unit)

open class KtObject{
      open fun _notification(): GodotNotification = godotNotification{}
    protected fun <T: KtObject> T.godotNotification(block: T.(Int) -> Unit ): GodotNotification = GodotNotification(block as Any.(Int) -> Unit)
}

open class A(): KtObject(){
    protected var counter: Int = 0
    override fun _notification() = godotNotification{ notification ->
        counter += 1
        println("A: $notification / Counter: $counter")
    }
}

open class B(): A(){
    override fun _notification() = godotNotification{ notification ->
        counter += 2
        println("B: $notification / Counter: $counter")
    }
}
class C(): B(){
    override fun _notification() = godotNotification{ notification ->
        counter += 3
        println("C: $notification / Counter: $counter")
    }
}

object KtClass {
    val stack = arrayOf(
        A()._notification().block,
        B()._notification().block,
        C()._notification().block
    )

    fun notification(c: C, notification: Int, reverse: Boolean){
        if(reverse){
            for (func in stack.reversed()){
                c.func(notification)
            } 
        } else {
            for (func in stack){
                c.func(notification)
            }
        }
    }
}

fun main() {
    KtClass.notification(C(), 13, false)
    println("")
    KtClass.notification(C(), 13, true)
}

@chippmann
Copy link
Contributor

@piiertho wasn't this resolved with #544?

@CedNaru CedNaru closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants