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

apparently no replacement for NOTIFICATION_APPLICATION_QUIT_REQUEST #64320

Closed
nonchip opened this issue Aug 12, 2022 · 12 comments
Closed

apparently no replacement for NOTIFICATION_APPLICATION_QUIT_REQUEST #64320

nonchip opened this issue Aug 12, 2022 · 12 comments

Comments

@nonchip
Copy link

nonchip commented Aug 12, 2022

SOLUTION

simply replace MainLoop.NOTIFICATION_APPLICATION_QUIT_REQUEST with Node.NOTIFICATION_WM_CLOSE_REQUEST.

but yeah this should probably stay open (so much for that :P) because it's poorly documented.


Godot version

4.0.dev

System information

N/A

Issue description

ever since PR #37317, MainLoop.NOTIFICATION_APPLICATION_QUIT_REQUEST does not exist anymore.

there seems to be a DisplayServer.WINDOW_EVENT_CLOSE_REQUEST now, but adapting the documentation like this:

func _notification(what):
    if what == DisplayServer.WINDOW_EVENT_CLOSE_REQUEST:
        print("quitting...")
        get_tree().quit() # default behavior

func _on_exit_button_pressed():
    get_tree().notification(DisplayServer.WINDOW_EVENT_CLOSE_REQUEST)

in the main scene's root script doesn't seem to trigger, neither via the "exit button" nor by closing the window (this just quits the game directly)

Steps to reproduce

N/A ; see above.

Minimal reproduction project

No response

@nonchip
Copy link
Author

nonchip commented Aug 17, 2022

after a bit more research, DisplayServer.window_set_window_event_callback(Callable(int event) callback, int window=0) exists, which is both HORRIBLY NAMED (seriously, window_set_window?!), and a pain to use.

sooo! to get the (mind you, super basic) feature of "do you want to exit?" back, this seems to be required now:

extends Node

# WindowEventHandling singleton

func _ready():
	DisplayServer.window_set_window_event_callback(_on_window_event)


# note, if this isn't callable, doesn't have the right arguments or just stops existing due to a scene change (that's why this is a singleton), the engine just doesn't do anything on any window event.
func _on_window_event(event: int) -> void:
	if event == DisplayServer.WINDOW_EVENT_CLOSE_REQUEST:
		exit()

func exit():
	# ask player, etc
	get_tree().quit()
# in your main menu scene or something:
func _on_exit_button_pressed():
	WindowEventHandling.exit()

like, i get the idea of "not every window is the only window", but then, why isn't this at least a signal on an actual window object instead of callback registration hell? there can also only be one callback per window, unlike a signal/notification, oh and yeah if you do it on e.g. your main menu code instead of a singleton, as soon as the callback vanishes because you freed the menu after going ingame/etc, it'll just literally do nothing, not close the window, not raise an error, just nothing.

oh also this approach would of course disable any od the engine's own window related event handling, not just for closing.

@nonchip
Copy link
Author

nonchip commented Aug 17, 2022

ok so after even more digging through undocumented code, ....

... insert drumroll ...

MainLoop.NOTIFICATION_APPLICATION_QUIT_REQUEST simply got renamed to Window.NOTIFICATION_WM_CLOSE_REQUEST and nobody ever mentioned it in any of the related issues/PRs/doc.

so no need for any of the above, just a doc fix, probably both in Window and anywhere in the turorials this common use case was mentioned.

also it looks like there's a bunch of signals for those kinda notifications on Window, so i guess one can go both the _notification and connect ways.

@Calinou
Copy link
Member

Calinou commented Aug 17, 2022

so no need for any of the above, just a doc fix, probably both in Window and anywhere in the turorials this common use case was mentioned.

Feel free to open a pull request for this 🙂

@nonchip
Copy link
Author

nonchip commented Aug 17, 2022

@Calinou i would but i have literally never contributed so far and uh dunno scared :P might look into it but can't promise anything yet. if i find the time/energy i'll do it, but then again i have a suspicion this might happen anyway as soon as the docs get updated for beta? especially since in the editor if i build it bleeding edge they're both documented, just not on the "latest" web docs or the tutorials/etc.

@lupoDharkael
Copy link
Contributor

I can do a PR to document the new API. I'll be able to work on it tomorrow.

@lupoDharkael
Copy link
Contributor

After researching a bit the source code I've found NOTIFICATION_WM_CLOSE_REQUEST does not pertain to Window but to Node (which Window inherits and thus making Window.NOTIFICATION_WM_CLOSE_REQUEST valid syntax).
As you can see the constant is already documented in Node: https://docs.godotengine.org/en/latest/classes/class_node.html#constants

Window has the signal close_requested which also serves this purpose which is documented too.

There is a section in the docs that document the existence of this constant and the information seems to be up to date https://docs.godotengine.org/en/latest/tutorials/inputs/handling_quit_requests.html

It looks like it is properly documented but it is confusing for users of Godot who used Godot 3.x, Maybe we should have an entry in the docs about migrating from Godot 3 to Godot 4 that mentions this and other changes. Having the old naming in the docs makes it searchable and may be a good solution for confused users migrating their projects.

@Zireael07
Copy link
Contributor

Maybe we should have an entry in the docs about migrating from Godot 3 to Godot 4 that mentions this and other changes

We definitely need such a docs page, for many other reasons (signals -> Callables, for instance)

@mojoyup
Copy link

mojoyup commented Jan 30, 2023

If` you just leave out the get_tree() part of the exit button code, it works. so just notification(DisplayServer.WINDOW_EVENT_CLOSE_REQUEST).

func _notification(what):
    if what == DisplayServer.WINDOW_EVENT_CLOSE_REQUEST:
        print("quitting...")
        get_tree().quit() # default behavior

func _on_exit_button_pressed():
    get_tree().notification(DisplayServer.WINDOW_EVENT_CLOSE_REQUEST)

@Mickeon
Copy link
Contributor

Mickeon commented Feb 12, 2023

CC @Calinou This could probably be mentioned in the Upgrading to Godot 4 page.

@nonchip
Copy link
Author

nonchip commented Feb 13, 2023

@mojoyup

Ifyou just leave out the get_tree() part of the exit button code, **it works**. so justnotification(DisplayServer.WINDOW_EVENT_CLOSE_REQUEST)`.

well Object.notification "notifies everything in the Object's class hierarchy", so if your self.notification works but SceneTree.notification doesn't, that would imply something in the node you're scripting there catches it, sadly didn't work for my use case like that, and kinda makes me wonder why Node.NOTIFICATION_WM_CLOSE_REQUEST worked for me instead, apparently there's different related notifications like that being used for various parts of the engine, probably triggering each other on the "DisplayServer-Window boundary"?

@akien-mga
Copy link
Member

SOLUTION

simply replace MainLoop.NOTIFICATION_APPLICATION_QUIT_REQUEST with Window.NOTIFICATION_WM_CLOSE_REQUEST.

but yeah this should probably stay open because it's not documented.

It's a Node notification: https://docs.godotengine.org/en/latest/classes/class_node.html#constants

@nonchip
Copy link
Author

nonchip commented Feb 23, 2023

SOLUTION

simply replace MainLoop.NOTIFICATION_APPLICATION_QUIT_REQUEST with Window.NOTIFICATION_WM_CLOSE_REQUEST.
but yeah this should probably stay open because it's not documented.

It's a Node notification: https://docs.godotengine.org/en/latest/classes/class_node.html#constants

yup, just not sure if the closing and archiving and "done" label might have been a bit "too fast" though, because we already figured that bit out months ago, i just forgot to change the original post (sorry), and this turned more into the "how to document it better" instead of the actual "feature missing" (because it never was, just "poorly" documented after godot learned about all the fancy new things windows can now do and had to throw some old concepts like "there's the one close request for the one main window" out).

also, just out of interest, why would a thing that feels like "can only happen to Windows" be in Node, do you happen to know some magic reason for that (like nodes holding a WM resource that's secretly able to "close" too), or is that just to sneak that const into the "default scripting namespace"?

so yeah if you want to keep this closed could you make sure it's still on some kinda "might be todo, look into"-thing that can be found by docs writing people in the very upcoming release that'll need that info, instead of just the rather final "Done, archived, closed" :)

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

No branches or pull requests

7 participants