Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Bug Fix - Cannot X out of MacroMenu #41

Merged
merged 8 commits into from
Oct 22, 2023
Merged

Conversation

rjwignar
Copy link

This fixes #22

Changes were tested using Visual Studio 2022 on Release 86 configurations

Summary of Changes

Modified the following files:

  • src/gui.h
    • Changed ShowMacroMenu() prototype to take pointer type parameter
  • src/gui.cpp
    • Adjusted ShowMacroMenu() implementation to match prototype changes
  • src/render.cpp
    • Changed Render() to stop calling gui::ShowMacroMenu() when Macro Menu is closed (when show_macro_menu set to false) and to end program if no other windows are open.

Reasons for Changes

Modifying ShowMacroMenu()

  • Although p_open is being passed by reference to ImGui::Begin(), it looks like p_open itself was being passed-by-value
  • I believe because of this, closing the Macro Menu would not actually modify show_macro_menu.
  • Modifying ShowMacroMenu() to take a pointer type parameter should ensure show_macro_menu will be modified when the Macro Menu is closed
  • I tested the original implementation of ShowMacroMenu() with my other changes but found the Issue would persist unless I made the above changes.

Changes to Render()

  • After experimenting, I observed that the main loop will keep calling Render(), so gui::ShowMacroMenu() would always be called even if the Macro Menu were closed.
  • In other words, the original implementation of Render() would immediately re-render (invoke ShowMacroMenu()) if the Macro Menu was closed.
  • Wrapping the gui::ShowMacroMenu() call in an if/else block ensures the Macro Menu will stay closed once the user clicks the close button.
  • The else statement allows the program to be shut off once the user exits the Macro Menu.

Conclusion

Please let me know if other changes are required.

Additionally, if this PR is approved and merged, I'd like this contribution to count towards my Hacktoberfest 2023 progress.
If the above changes are approved and merged, could the hacktoberfest-accepted label be added to the PR/MR?

Thank you for considering these changes.

@GudBoiNero GudBoiNero added hacktoberfest-accepted bug Something isn't working good first issue Good for newcomers labels Oct 22, 2023
@GudBoiNero
Copy link
Owner

Great job on this PR rjwignar! I really appreciate the help! And I added the hacktoberfest-accepted tag as requested.

Thanks!

@GudBoiNero GudBoiNero merged commit 073b847 into GudBoiNero:beta Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants