Skip to content

Debugger: Rename debugger widgets to views and update UI layout format #12440

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

Merged
merged 4 commits into from
Apr 14, 2025

Conversation

chaoticgd
Copy link
Contributor

@chaoticgd chaoticgd commented Mar 19, 2025

Description of Changes

The DebuggerWidget class has been renamed to DebuggerView. All subclasses have also been renamed.

This makes three changes to the format:

  • The version properties are now in camelCase like the rest of the properties (instead of snake_case).
  • The hash algorithm used for determining if the default layouts need to be recreated has been changed.
  • A new id property is used to sort dock widgets with the same display name instead of using their unique name.

I've also fixed a bug where the UI layouts may have been saved unnecessarily after loading them.

Rationale behind Changes

  • I believe the view terminology is more appropriate. Everyone already calls them views.
  • I thought using MD5 for the default layout hash was a bit silly when a simple non-cryptographic hash would more than suffice.
  • I bundled together these changes so that users will only have their layouts deleted once instead of twice.

Suggested Testing Steps

  • Check the diff, see if I've missed anything, or screwed up anything with find/replace.
  • Make sure the debugger still works, in particular loading layouts.
  • Existing layouts will be deleted (sorry).

@chaoticgd chaoticgd force-pushed the layout_version branch 3 times, most recently from 18a0ec3 to 91e6840 Compare March 19, 2025 23:12
@chaoticgd chaoticgd marked this pull request as draft March 20, 2025 16:56
@chaoticgd chaoticgd force-pushed the layout_version branch 2 times, most recently from 7aa1d08 to a2ccf02 Compare April 12, 2025 17:33
@chaoticgd chaoticgd changed the title Debugger: Revise file format for UI layouts Debugger: Rename debugger widgets to views and update UI layout format Apr 12, 2025
@chaoticgd chaoticgd marked this pull request as ready for review April 12, 2025 18:05
@chaoticgd
Copy link
Contributor Author

Ready for review now. I've changed it quite a lot.

@chaoticgd chaoticgd force-pushed the layout_version branch 3 times, most recently from 2c55590 to 6594884 Compare April 12, 2025 22:43
Copy link
Contributor

@kamfretoz kamfretoz left a comment

Choose a reason for hiding this comment

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

Debugger still works great.

image

@recursean
Copy link
Contributor

I tested this on Linux and things look good. Debugger and layouts seem to work as expected.

Only thing I noticed when looking at the diff was that you didn't rename Debugger/Docking/NoLayoutsWidget

@chaoticgd
Copy link
Contributor Author

Only thing I noticed when looking at the diff was that you didn't rename Debugger/Docking/NoLayoutsWidget

That's okay, it's not like the others.

@chaoticgd
Copy link
Contributor Author

chaoticgd commented Apr 13, 2025

Would this cause more work for translators? Like, would we need to batch change the translations to match the new type names somehow too? I'm not sure how that would work.

@recursean
Copy link
Contributor

I barely know anything about the translation stuff but... I ran .github/workflows/scripts/common/update_base_translation.sh against your changes and after running, pcsx2-qt/Translations/pcsx2-qt_en.ts was updated with the new class names replacing the old. So, seems like everything should be normal for the translators?

@F0bes
Copy link
Member

F0bes commented Apr 14, 2025

It's automatically managed by cron jobs. Don't worry about it.

@F0bes F0bes merged commit 0ffb6d6 into PCSX2:master Apr 14, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants