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

The Dreaded ConfigCommand.py Refactor [AARD-1764] #1107

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

BrandonPacewic
Copy link
Member

@BrandonPacewic BrandonPacewic commented Aug 22, 2024

I am happy to report that this file is now less than 240 lines. 😊

  • Set a default size for the main config panel:
    • This was done to solve an issue with the joint and gamepiece config tables not showing up correctly.
  • Removed unused handler code.
  • Remove unused code within ConfigCommand.py.
  • Removed preselect handler as it simply was not working as originally intended.
  • Updated the help icon redirect to be directly to the exporter codelab.
  • Changed the default location for export to be local download.
  • Moved handler creation to the top of create:
    • Results in more stable behaviour if we crash later when adding joints, wheels and gamepieces.
  • Added PersistentEventHandler class that will auto add a handler to the global list to keep it from falling out of scope.
  • Added better typing with adsk.fusion.Design.cast(adsk.core.Application.get().activeProduct) for the design getter call.
  • Fixed a persistent issue from Catch When No Design is Selected #1099.

JIRA Issue

* dev: (28 commits)
  Removed console logs
  Fixed all issues
  Debug lines
  canOPFS check
  Reverted change to settings modal
  Remove comment ¯\_(ツ)_/¯
  reverted accidental sim changes
  Organized subsystem config differently
  Remove mass icons
  Better generationing and removing
  Generation fix
  Fix variable name typo
  Update conversion function typing
  Synthesis Exporter now tracks Fusion's unit system
  setting scaling fix
  Merge conflict fix
  Lots of documentation
  Remove old import
  add catch for no robot being avaliable
  Tooltips!!!
  ...
@BrandonPacewic BrandonPacewic added exporter refactor The most important part of software development. labels Aug 22, 2024
@BrandonPacewic BrandonPacewic self-assigned this Aug 22, 2024
@BrandonPacewic BrandonPacewic marked this pull request as ready for review August 23, 2024 16:27
@BrandonPacewic BrandonPacewic requested review from PepperLola and azaleacolburn and removed request for a team August 23, 2024 16:27
Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

I like the default export location change. Nice job getting it down to so few lines 🥇

One thing I noticed is that it no longer includes the version number in the exported file name. Not sure if this was intentional and I don't mind either way, just something to note.

Copy link
Member Author

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

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

One thing I noticed is that it no longer includes the version number in the exported file name. Not sure if this was intentional and I don't mind either way, just something to note.

Unfortunately, this was not intended. 🫤

Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

Still works and includes the full name and version this time. I will say that previously it would only concatenate the name and version with an underscore and the rest would be the raw name with spaces (i.e. Utility Knife_v2.mira instead of the Utility_Knife_v2.mira that happens now). I don't care either way and this is definitely less important than including the version number and full name.

Copy link
Member Author

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

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

I will say that previously it would only concatenate the name and version with an underscore and the rest would be the raw name with spaces (i.e. Utility Knife_v2.mira instead of the Utility_Knife_v2.mira that happens now). I don't care either way and this is definitely less important than including the version number and full name.

Yea this was on purpose. Was simpler to write it this way and I figured it was fine to replace all spaces with underscores.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter refactor The most important part of software development.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants