-
Notifications
You must be signed in to change notification settings - Fork 3k
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
console.lua: allow subsequent mp.input calls #15796
base: master
Are you sure you want to change the base?
Conversation
e64e29f
to
be6538f
Compare
Download the artifacts for this pull request: |
if repl_active and input_caller and script_name ~= input_caller then | ||
mp.commandv('script-message-to', input_caller, 'input-event', | ||
'closed', utils.format_json({line, cursor})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better suggestions welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a stack with the state, and restore whatever was/is open after select ended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rejected in #10282 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the other way, to immediate go to user requested input, but go back to previous state, if requested (closed without select). i.e. allow nested menus in menu type situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it strange to go back to the previous state when it was invoked by a completely different script doing something completely different? I think it should still send closed
to different scripts like in this PR. Restoring the previous state from the same script is a separate functionality that can be added later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think close without select or indeed another dedicated function could achieve that.
As for the rest, I don't think other scripts should be able to interrupt the selection that is being made, with implicit way. If it is invoked as "nested" select we can support that to provide "menu" like interface, but I don't see others nuking already open select. That's why I feel that it is hacky, because we allow it to open and send some implicit closed events, but only if script is different, it's weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair though I think one reasonable use case for it is to try out script-binding select/...
bindings in the console, this was actually requested in #14087 (comment) (and I misunderstood it as referring to the input queue at that time too).
This doesn't actually fix calling |
Currently using mp.input while the console is open doesn't do anything. We can allow using mp.input multiple times in a row by keeping the console open, and sending a closed event only when the previous calling script is different from the current one. If it is the same, sending a closed event would unregister the new input-event listener, so it is just never sent in this case as there is no sane way to do it. This is useful e.g. to try out select script bindings in the console, or to switch to a different mp.input script binding with a key binding without having to close the current one. Calling input.select() again within input.select()'s submit handler needs to be fixed separately because with select the console is immediately closed on submit (367a6b5), so input.lua receives submit, requests a new selection, and receives closed wich unregisters the handler of the new selection. Just not calling mp.unregister_script_message("input-event") fixes it; its only purpose was to allow garbage collection of variables defined in the scope using mp.input. Fixes mpv-player#15795.
be6538f
to
8f01011
Compare
I just removed the |
Currently using mp.input while the console is open doesn't do anything. We can allow using mp.input multiple times in a row by keeping the console open, and sending a closed event only when the previous calling script is different from the current one. If it is the same, sending a closed event would unregister the new input-event listener, so it is just never sent in this case as there is no sane way to do it.
Fixes #15795.