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

GDScript add with context management syntax #3490

Open
briansemrau opened this issue Nov 1, 2021 · 5 comments
Open

GDScript add with context management syntax #3490

briansemrau opened this issue Nov 1, 2021 · 5 comments

Comments

@briansemrau
Copy link

briansemrau commented Nov 1, 2021

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: #3486 (comment)

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 godotengine/godot#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
Copy link
Member

Calinou commented Nov 1, 2021

See also #623.

@zinnschlag
Copy link

@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
Copy link

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
Copy link

theraot commented Nov 11, 2021

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
Copy link

r-owen commented Jun 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants