godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.08k stars 69 forks source link

GDScript add `with` context management syntax #3490

Open briansemrau opened 2 years ago

briansemrau commented 2 years ago

Describe the project you are working on

A game that uses mutexes extensively, but this applies to any project requiring context management such as file open/close.

Describe the problem or limitation you are having in your project

Objects that require cleanup after use can be tricky to use safely. Example from #3486:

var my_mutex := Mutex.new()

func thread_safe() -> Error:
    my_mutex.lock()
    for x in data:
        var err = x.data_do_something()
        if err:
            return err  # !!! forgot to unlock !!!
    my_mutex.unlock()
    return OK

This applies to file open/close, and possibly other situations. See comment: https://github.com/godotengine/godot-proposals/issues/3486#issuecomment-955995217

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add the pythonic with syntax (see https://www.python.org/dev/peps/pep-0343/)

This way, the above code could be reimplemented like so:

var my_mutex := Mutex.new()

func thread_safe() -> Error:
    with my_mutex:
        for x in data:
            var err = x.data_do_something()
            if err:
                return err  # !!! forgot to unlock !!!
        return OK

(This conflicts with #623)

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

There are several ways to implement this. Here's a suggestion:

Add NOTIFICATION_ENTER_CONTEXT, NOTIFICATION_EXIT_CONTEXT to Object. When the object is passed to the with scope, call this notification immediately. When the scope expires, notify EXIT_CONTEXT.

In Mutex, File, support these notifications by adding a call to lock/unlock, open/close.

To ensure that contexts are released in case of an error, I expect that there would have to be much more intrusive changes to GDScript.

This would supercede #3486 (and associated PR https://github.com/godotengine/godot/pull/54444).

If this enhancement will not be used often, can it be worked around with a few lines of script?

From #3486, a hacky way to do this, using Mutex as an example:

class MutexLock extends RefCounted:
    var mutex: Mutex

    func _init(m: Mutex):
        mutex = m
        mutex.lock()

    func _notification(what):
        if what == NOTIFICATION_PREDELETE:
            mutex.unlock()
var my_mutex := Mutex.new()

func thread_safe() -> Error:
    if true:
        var lock = MutexLock.new(mutex)
        for x in data:
            var err = x.data_do_something()
            if err:
                return err  # !!! forgot to unlock !!!
        return OK

Is there a reason why this should be core and not an add-on in the asset library?

GDScript is part of core. The workaround described above is not as performant.

Calinou commented 2 years ago

See also https://github.com/godotengine/godot-proposals/issues/623.

zinnschlag commented 2 years ago

@Calinou This are actually two completely different withs. The one here is about cleanup after leaving a scope. The one you linked is about a shortcut for accessing member variables/methods. Other than both use the same keyword they have nothing in common.

zinnschlag commented 2 years ago

File would have to be handled somewhat differently, since you need to pass one or more arguments. Could still use the same mechanism thought. I imagine something like this:

func file_demo() -> Error:
    with var f = File.new("some filename", File.WRITE):
        f.store_string("something")
    return OK

In this example NOTIFICATION_ENTER_CONTEXT would remain unused but NOTIFICATION_EXIT_CONTEXT would be used for closing. This proposed syntax also includes a way to add a named variable. This would be equivalent of the Python keyword as, which is used for a different purpose in GDScript and wouldn't be a good idea therefore.

theraot commented 2 years ago

About the conflict with that other proposal that also uses with (which, by the way, reminds me of Visual Basic), this one could use using following C# footsteps.

r-owen commented 3 weeks ago

I am hoping this will be considered again. Having a reliable way to clean up after a block of code is a big win. Python uses:

  with Foo(...) as foo:
      ... protected code
      foo will be cleaned up even if the code in this block fails
      e.g. a file will be closed, a mutex freed

and GGScript does seem to be largely based on Python. But anything that will do the job will be welcome.