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

update confirm to use overlays and Lua-based confirmation prompts #914

Merged
merged 4 commits into from
Jan 2, 2024

Conversation

myk002
Copy link
Member

@myk002 myk002 commented Jan 2, 2024

@myk002 myk002 merged commit 478b5f6 into DFHack:master Jan 2, 2024
10 checks passed
@myk002 myk002 deleted the myk_confirm branch January 2, 2024 03:57
end
end

if not next(registry) then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should try to avoid checks like this because it makes development harder. Making any changes to the ConfirmConf invocations below would require manually clearing registry or restarting DF. Previously I think reloading the plugin (or the plugin lua module) was enough to apply changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be better to take out the line that checks for duplicate IDs in ConfirmConf:init()? That was intended to catch copy paste errors, but if it were taken out, then the conf definitions would get automatically refreshed on every run

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested change in 1bf7996

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
2 participants