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

add message interface #31

Merged
merged 1 commit into from
Jan 14, 2024
Merged

add message interface #31

merged 1 commit into from
Jan 14, 2024

Conversation

tsl0922
Copy link
Owner

@tsl0922 tsl0922 commented Jan 14, 2024

Replaces #30.

example usage:

test.lua:

local utils = require('mp.utils')
local msg = require('mp.msg')

mp.register_script_message('menu-get-reply', function(data)
	msg.info('reply: ' .. data)
end)

local menu = {
	title='recent',
	type='submenu',
	submenu={
		{title='foo', cmd='show-text foo'},
		{title='bar', cmd='show-text bar'},
	},
}

mp.register_script_message('menu-ready', function()
	mp.commandv('script-message-to', 'dyn_menu', 'get', 'audio-devices', mp.get_script_name())
	mp.commandv('script-message-to', 'dyn_menu', 'update', 'recent', utils.format_json(menu))
	mp.commandv('script-message-to', 'dyn_menu', 'update', 'chapters', utils.format_json(menu))
end)

input.conf:

_           ignore                            #menu: Audio Devices #@audio-devices
_           script-binding recentmenu/open    #menu: Recent #@recent
_           ignore                            #menu: Chapters #@chapters

@tsl0922 tsl0922 mentioned this pull request Jan 14, 2024
2 tasks
@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

This is much better than my implementation.

@tsl0922 tsl0922 force-pushed the message branch 2 times, most recently from f7d8e19 to e1023a3 Compare January 14, 2024 05:07
@tsl0922 tsl0922 mentioned this pull request Jan 14, 2024
@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

There is another issue here, you render the titles passed by other scripts as titles in the menu, but this will cause the specified titles in the input.conf file to be overwritten.

For example, input.conf:

_           ignore    #menu: 最近播放 #@recent

image

This behavior is not what I want. Adding additional detection that only force overwrite when the corresponding title in the input.conf file is empty.

@tsl0922
Copy link
Owner Author

tsl0922 commented Jan 14, 2024

As a convenience, if you don't want to override menu title and type, omit the corresponding field in json.

@tsl0922 tsl0922 force-pushed the message branch 2 times, most recently from 326ef71 to 876d304 Compare January 14, 2024 06:30
@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

As a convenience, if you don't want to override menu title and type, omit the corresponding field in json.

This is just what I wanted.

@tsl0922 tsl0922 force-pushed the message branch 2 times, most recently from 66d6ea8 to e910914 Compare January 14, 2024 06:47
@tsl0922 tsl0922 merged commit 0adb7c3 into main Jan 14, 2024
2 checks passed
@tsl0922 tsl0922 deleted the message branch January 14, 2024 07:14
@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

The last commit disrupted the JSON work of other scripts.

tsl0922 added a commit that referenced this pull request Jan 14, 2024
@tsl0922
Copy link
Owner Author

tsl0922 commented Jan 14, 2024

Pushed a fix for it.

I'm new to lua 🤣 .

@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

Pushed a fix for it.

I'm new to lua 🤣 .

Not a completely correct fix, there is a syntax issue with determining that the table in the script is empty.

@tsl0922
Copy link
Owner Author

tsl0922 commented Jan 14, 2024

Well, I just removed the check for empty table, after I realized that I have to iterate it to check if it is empty.

In fact ,there maybe other cornercases here, I'm not going to validate all of them, because the C plugin will ignore them.

@dyphire
Copy link
Contributor

dyphire commented Jan 14, 2024

Well, I just removed the check for empty table, after I realized that I have to iterate it to check if it is empty.

In fact ,there maybe other cornercases here, I'm not going to validate all of them, because the C plugin will ignore them.

#32

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