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

spectate.lua: alternative way to make config public #5298

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

TymurGubayev
Copy link
Contributor

No description provided.

@TymurGubayev
Copy link
Contributor Author

another alternative: public get_config() and get_save_state()

@myk002
Copy link
Member

myk002 commented Feb 22, 2025

I refactored gui/spectate to do read-only lookups of plugin state and to use the public API for setting the values. That way the cpp part of the plugin stays synced with the Lua

@TymurGubayev
Copy link
Contributor Author

I see you are now doing dfhack.run_command('spectate', 'set', cfg_elem, text). But parse_commandline is already public and would have the same effect(*). Using it would avoid one layer of indirection.

Also, to be really really sure, we should read the config state on the UI side after dfhack.run_command/parse_commandline is executed, in case it is rejected for whatever reason, and update the UI accordingly.

That said, this is all not important. Please feel free to close this PR.

(*) if there are exceptions depends on what dfhack.run_command does, but that could be solved by adding a pcall on the UI script side.

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