-
-
Notifications
You must be signed in to change notification settings - Fork 524
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
fix(launcher): 🐛 Fix installation popup #2356
Conversation
How does this work? |
This change makes it so instead of creating a copy of the release channel version, it is actually mutable. This should save memory overall I believe right? |
Basically i refactored away some logic for the popup, and in doing so we now need to call some code in a closure. Closures often means aliasing issues so I usually clone the variables needed. This time though we needed to mutate the variable ( |
Yea that's what the diff says, but the question is why it's done. It's clearly a fix going by the diff message, so this somehow has to fix something |
@KyleCarr this is not about memory saving. It was actually a bug caused by a refactoring. Most of the code in ALVR doesn't care about memory usage, apart some crucial paths while dealing with video buffers |
Could've found that out myself tbh, I just keep getting blinded a bit by the lack of context from diff views |
Yeah I would have never found this myself even staring at the code for hours. It was reported in the Discord server. It slipped my testing apparently. |
Well yea, finding the initial bug, but finding the reason for the change is reasonably doable I think |
No description provided.