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

Attach script at root CanvasLayer, so you can run DebugMenu.style #16

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

d10sfan
Copy link
Contributor

@d10sfan d10sfan commented Sep 15, 2023

This fixes #15

Attached the script to the root CanvasLayer and re-attached the correct nodes.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

Thanks! Congratulations for your first merged pull request 🎉

@Calinou Calinou merged commit eeb2b60 into godot-extended-libraries:master Sep 15, 2023
@d10sfan d10sfan deleted the toggle-menu-fix branch September 15, 2023 18:37
@Calinou
Copy link
Member

Calinou commented Sep 15, 2023

I noticed this breaks _on_visibility_changed():

E 0:00:00:0835   debug_menu.gd:51 @ @style_setter(): Error calling from signal 'visibility_changed' to callable: 'Control::_on_visibility_changed': Method not found.
  <C++ Source>   core/object/object.cpp:1082 @ emit_signalp()
  <Stack Trace>  debug_menu.gd:51 @ @style_setter()
                 debug_menu.gd:140 @ _input()

Can you look into fixing this in a follow-up PR?

Alternatively, we can get rid of the entire method as it doesn't seem to do much nowadays. I've tried removing it locally and it doesn't seem to impact graphs negatively when you toggle them just after starting the project.

@d10sfan
Copy link
Contributor Author

d10sfan commented Sep 15, 2023

Ah good catch! I've fixed that in #17

The signal has been moved and I can see that it's being called and there's no more error.

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

Successfully merging this pull request may close these issues.

README about programatically toggle the menu isn't correct
2 participants