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

Assorted fixes #1721

Merged
merged 7 commits into from
Jul 3, 2022
Merged

Assorted fixes #1721

merged 7 commits into from
Jul 3, 2022

Conversation

J5lx
Copy link
Member

@J5lx J5lx commented Jul 2, 2022

This PR is a collection of various unrelated fixes and enhancements. I don’t think any of them have been (formally) reported. Here’s a list of the issues:

  • An issue where the project recovery dialog was not modal, which caused it to appear behind the main window on my system
  • Some runtime warnings about missing object names for the toolbars. This didn’t seem to cause any actual issues, but I figured I should fix it anyway
  • An issue that caused the undo/redo shortcuts not to work under certain circumstances, for example right after program startup
  • An issue where the translation context was set incorrectly for some strings due to inappropriate use of QObject::tr()
  • An issue where Pencil2D ignored even the --help and --version options in release builds when another instance was already running
  • A memory leak related to object data

I also took another look at the setlocale() workaround for #940 because I always felt like there must be a better solution. And I was right! Not only that, but the Qt devs already implemented it some time ago in 5.14 – because as it turns out, this was a bug in Qt all along. I’ve verified that the bug is indeed gone in current Qt versions and updated the code accordingly. I also updated it to use the C locale instead of US English (because the latter might not always be available) and moved the code so that it is run after QCoreApplication has been initialised, as recommended in the documentation.

J5lx added 7 commits June 29, 2022 02:27
The dialog was being shown from within MW2's showEvent, but that event is called
just *before* the window is visible, hence why the dialog wasn't actually modal.
The enable/disable code was written under the assumption that undo/redo could
only be triggered from the menu, but did not take keyboard shortcuts or the
recently added toolbar into account.
@J5lx J5lx added the Bug label Jul 2, 2022
Copy link
Member

@scribblemaniac scribblemaniac left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@scribblemaniac scribblemaniac added the 🔹 Minor PR (only one reviewer required) label Jul 3, 2022
Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM too, I will merge the changes now

@MrStevns MrStevns merged commit 7b910c4 into pencil2d:master Jul 3, 2022
@J5lx J5lx deleted the fixes/misc branch July 3, 2022 08:23
@MrStevns MrStevns added this to the v0.6.7 milestone Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🔹 Minor PR (only one reviewer required)
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants