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

Enable file system watcher on mobile #704

Merged
merged 1 commit into from
May 2, 2024

Conversation

andybak
Copy link
Contributor

@andybak andybak commented May 2, 2024

Let's see if this works now

@mikeskydev
Copy link
Member

Please can this be tested on Q1!

@andybak
Copy link
Contributor Author

andybak commented May 2, 2024

Please can this be tested on Q1!

I don't currently have access to a Quest 1. Do you?

@mikeage
Copy link
Member

mikeage commented May 2, 2024

I can test Q1 tonight, just please tell me what specifically to look out for

@andybak
Copy link
Contributor Author

andybak commented May 2, 2024

This fixes the bug on Quest 3 at least. @mikeage - I think we just need to test that it doesn't make anything worse on Quest 1.

Try using the Media Library browser and see if it does anything weird.

If it's working, then you should be able to add new images into the media library with Open Brush running and see them appear as previews in the Media Library panel instantly.

If it's not working but not breaking stuff then they just won't appear.

If it's breaking stuff then... dunno. Weird stuff will happen?

@andybak andybak marked this pull request as ready for review May 2, 2024 11:34
@andybak andybak self-assigned this May 2, 2024
@andybak andybak requested review from mikeage and mikeskydev May 2, 2024 11:34
@andybak andybak added the bugfix Something has been fixed label May 2, 2024
@mikeage
Copy link
Member

mikeage commented May 2, 2024

Works on Q1. Pushing or deleting an image immediately brings up the "loading images" screen and then the new one is seen automatically.

@mikeskydev , do we need to test other platforms, Pico, etc.? Or did you ask about Q1 because that's the only one you don't have.

@mikeskydev
Copy link
Member

I was mainly concerned about the performance implications of the watcher, but that sounds fine!

@mikeage
Copy link
Member

mikeage commented May 2, 2024

it didn't seem to cause anything (assuming it's inotify under the hood, it should be zero impact)

@mikeage
Copy link
Member

mikeage commented May 2, 2024

I'm approving, but let's not merge until I fix the DMG build

@mikeage
Copy link
Member

mikeage commented May 2, 2024

this is ready to go, as soon as the build on main finishes

@andybak andybak merged commit abe6e55 into main May 2, 2024
42 checks passed
@andybak andybak deleted the sh/feature/enable-filesystem-watcher-on-mobile branch May 2, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Something has been fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants