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

Menu script-message changes #653

Merged
merged 4 commits into from
Sep 30, 2023

Conversation

christoph-heinrich
Copy link
Contributor

@christoph-heinrich christoph-heinrich changed the title Menu message changes Menu script-message changes Sep 28, 2023
@tomasklaen
Copy link
Owner

That was the next thing on my to-do list :)

The only thing it's missing is keeping track of currently opened menu on the user-data/uosc/menu/type property, as using stuff like update-menu depends on it.

@tomasklaen
Copy link
Owner

Also, all the menu toggling bindings (menu, subtitles, ...) need to be updated as they no longer toggle.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 29, 2023

The only thing it's missing is keeping track of currently opened menu on the user-data/uosc/menu/type property, as using stuff like update-menu depends on it.

Scripts that open their menu already know the type, no need to query a property to update their own menu.
I know it works because I already adapted quality-menu and memo. (memo has an open PR and quality-menu will be pushed once this is in)

Also, all the menu toggling bindings (menu, subtitles, ...) need to be updated as they no longer toggle.

They do still toggle, I made sure of that before submitting it.
Unless I missed one, all builtin menus start with if Menu:is_open(menu_type) then Menu:close() return end.

@tomasklaen
Copy link
Owner

Wait I don't understand now. You need to know if your menu is open to send either open-menu or update-menu message. So how can scripts know their menu is open if they can't get user-data/uosc/menu/type? The property check is not just to get the type, but to check if any menu is currently open, and if it's still the menu opened by the script.

I thought that the rework is supposed to implement @po5's requests that:

  • update-menu should no longer open menus
  • open-menu should no longer toggle

which I agree with. But then you need to be able to check what menu and of what type is currently open to re-implement these functionalities.

Also, menu-close should accept type as an optional parameter, and if it's empty, close any opened menu.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 29, 2023

So how can scripts know their menu is open if they can't get user-data/uosc/menu/type?

on_close can be used to know if the menu has been closed from the uosc side, so scripts can keep track of if their menu is currently open or not.

E.g. in quality-menu settting on_close and reacting to it.

@tomasklaen
Copy link
Owner

on_close can be used to know if the menu has been closed from the uosc side, so scripts can keep track of if their menu is currently open or not.

True, that is a viable alternative. But imo accessing a user-data prop is easier than setting up a on_close message listener and having a global flag that tracks it.

Regardless, I recall this being a requested feature (though I can't remember context anymore), it feels useful for scripts to be able to detect open menus, I think we should provide it, and it feels like it should be part of this PR. Since these are potentially breaking changes, I want to squash it into one feat(api)!: menu script-message changes commit with all of them listed in the description so that it's easier for people to inspect and adapt their code.

@christoph-heinrich
Copy link
Contributor Author

Added the new property.

@tomasklaen tomasklaen merged commit 512281a into tomasklaen:main Sep 30, 2023
tam1m pushed a commit to tam1m/uosc that referenced this pull request Oct 2, 2023
- `open-menu` now only opens the menu, while closing any existing one first, even if it has the same `type`.
- `update-menu` will only update a currently opened menu of the same type. If no menu is open, or current menu's type is different, it doesn't do anything.
- `close-menu [type]` can be used to close any currently opened menu when called without a `[type]` argument, or only a menu of `[type]` type.
- uosc now keeps track of a currently opened menu type on the `user-data/uosc/menu/type` property, accessible via `mp.get_property_native('user-data/uosc/menu/type')`. This property is `nil` if no uosc menu is opened.

This is to achieve a predictable and granular control of menus with no implicit magic going on in the background.

The main difference is that `open-menu` can no longer be used to toggle the menu. You have to implement toggling manually by calling `open-menu` or `close-menu [type]` when appropriate. You can check if your menu is still opened either by getting or observing the `user-data/uosc/menu/type` property, or using the `on_close` menu callback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants