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 information about gui module requirement for overlay to load #4896

Closed

Conversation

wiktor-obrebski
Copy link
Contributor

Without gui required, overlay module will not appear on the overlays list, which is confusing for the dev.

@@ -263,6 +263,8 @@ the ``Overlays`` tab.
(``--@ module = true``) and it does not have side effects when loaded as a
module (i.e. you check ``dfhack_flags.module`` and return before executing
any statements if the value is ``true``)
#. you script import `gui` module, even if it is not use it directly:
``local gui = require('gui')``
Copy link
Member

Choose a reason for hiding this comment

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

are you sure this is true? what situation did you see this with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without importing gui module I was not able to see the notes overlay in the control panel overflow list.
Can you try to recreate the issue with empty overlay class?

Copy link
Member

@myk002 myk002 Sep 1, 2024

Choose a reason for hiding this comment

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

idle-crafting, for example, has an overlay widget that does not require('gui')

Overlay files usually need gui because they reference one of the gui.FRAME_* identifiers or they create a gui.ZScreen, but not all overlays do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

idle-crafting require gui.widgets which itself require gui. I will make some test to confirm if this is the case.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with Myk - this should not be the case. If this is the case, that probably means importing the gui module has some side effects, and we should fix that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can not repeat it, so I closing the PR.
Maybe there was some another factor that I just did not understand earlier.

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

Successfully merging this pull request may close these issues.

3 participants