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

Almost all uses of experimental flag are removed #620

Closed
wants to merge 28 commits into from

Conversation

andybak
Copy link
Contributor

@andybak andybak commented Jan 31, 2024

  1. STL/WRL exports still use the flag. Import/export improvements #573 fixes this
  2. The experimental setting still handles brushes - this essentially makes the setting an "experimental brush" setting
  3. To make room on Labs for 3 new buttons I've hidden Poly, YouTube and Twitch. I think these do nothing currently.

…ed mode.

Also remove some dead code from "ExportAll" which we removed in 2.4
Copy link
Member

@mikeage mikeage left a comment

Choose a reason for hiding this comment

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

Looks like a great idea and an important step for 3.0.

@mikeskydev , can you comment on the youtube et al buttons

@mikeage mikeage added the enhancement Feature added label Feb 23, 2024
@mikeage
Copy link
Member

mikeage commented Feb 25, 2024

Is this waiting for anything (we addressed the streaming in discord, I believe)

@andybak andybak marked this pull request as draft February 25, 2024 15:08
@andybak
Copy link
Contributor Author

andybak commented Feb 25, 2024

I originally scoped this as "get this out the door before I go on holiday". I realised that shipping (even to beta) when I was about to be unavailable was a bad idea.

So there's a couple of things I can tweak now I'm not trying to fit it into a single day's work. The wording for the settings button ui can be improved.

And I can document the new settings that arise out of this. I'll do that in the next couple of days.

@andybak
Copy link
Contributor Author

andybak commented Feb 28, 2024

New config file settings:

Flags/AdvancedKeyboardShortcuts
Flags/SkipIntro

@andybak andybak marked this pull request as ready for review February 28, 2024 14:03
@andybak
Copy link
Contributor Author

andybak commented Feb 28, 2024

These translations are by GPT but it's a fairly simple change so it's hopefully correct.

@andybak
Copy link
Contributor Author

andybak commented Feb 28, 2024

So merging this before #573 leaves one small anomaly - STL/WRL exports still rely on the experimental mode setting which the UI claims only affects brushes.

However I don't think many people care about STL or WRL exports so that is probably fine.

@mikeskydev
Copy link
Member

Would it be possible to now load experimental brushes without restarting the app, by reloading the manifests?

@andybak
Copy link
Contributor Author

andybak commented Mar 19, 2024

Would it be possible to now load experimental brushes without restarting the app, by reloading the manifests?

Yes. I think so. I'll investigate.

Would you prefer that was included in this PR?

@mikeskydev
Copy link
Member

Would you prefer that was included in this PR?

Yes please!

@andybak andybak marked this pull request as draft March 19, 2024 16:58
@mikeskydev
Copy link
Member

Looks like you would need to call BeginReload() in BrushCatalog, EnvironmentCatalog etc when updating the manifest. Can see this in App.cs when the initial state load happens.

@andybak
Copy link
Contributor Author

andybak commented Mar 27, 2024

@mikeage @mikeskydev Any last thoughts before merging this? There's been some changes since the last review so I wanted to check.

It would be nice to let this sit in beta for a while to shake out any unexpected issues so if possible I'd like to merge it when there's unlikely to be a hotfix or some other reason for us to do an interim release.

@mikeage
Copy link
Member

mikeage commented Mar 27, 2024

The fact that we've done three releases in less than 48 hours says nothing about the likelihood of a 4th.

I'm fine with it from a release management POV. No opinions from a technical POV. But we definitely have a lot of open stuff, and it'd be good to start closing some...

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

Successfully merging this pull request may close these issues.

None yet

3 participants