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

Fully reload the instance list once the folder is changed #1589

Merged
merged 1 commit into from Nov 23, 2023

Conversation

Trial97
Copy link
Member

@Trial97 Trial97 commented Aug 31, 2023

fixes #1584

@Trial97 Trial97 added bug Something isn't working simple change changelog:fixed A PR that appears under "Fixed" in the changelog labels Aug 31, 2023
@Trial97 Trial97 changed the title Fully reload the instacne list once the folder is changed Fully reload the instance list once the folder is changed Aug 31, 2023
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
TayouVR
TayouVR previously approved these changes Aug 31, 2023
@TheKodeToad
Copy link
Member

All of the instances are removed for me

@TayouVR
Copy link
Member

TayouVR commented Sep 1, 2023

it works for me...

@Trial97
Copy link
Member Author

Trial97 commented Sep 1, 2023

All of the instances are removed for me

I want to try to reproduce this, can you describe the steps?

@TheKodeToad
Copy link
Member

I mean without this PR it's already removing instances for me

@Trial97
Copy link
Member Author

Trial97 commented Sep 1, 2023

Well obviously that already happens, what doesn't happen is:

  • if you have two instance folders and have an instance with the same id
  • switch between them in the settings menu( without restarting the launcher)
  • trying to start the instance with the same name would result in using the old one if the previous path is still valid, if the instance is no longer there, you will get the same prompt as in the linked issue

My test method for this was:

  • open prism
  • go to prism root folder
  • rename the instances folder to instances2
  • go into prism settings and change the folder to instances2
  • try to start an instance

@TayouVR
Copy link
Member

TayouVR commented Sep 1, 2023

huh, after some more testing....
This PR still works perfectly.
But on the develop branch build I have installed it just crashes... without any obvious indicator of what is happening.. might run a debugger on it at some point, though I have like no time at all right now and for the coming days
Didn't test the ID thing specifically, but yeah....

EDIT: reworded it a tiny bit to make it more clear

@Trial97 Trial97 requested a review from TayouVR September 1, 2023 12:37
@Trial97 Trial97 dismissed TayouVR’s stale review September 1, 2023 12:38

Mentione of some crash with current implementation

@Trial97
Copy link
Member Author

Trial97 commented Sep 1, 2023

EDIT: reworded it a tiny bit to make it more clear

I'm confused now does this PR crashes or not?
I removed your approval because I thought that the crash was related to my PR. Can you approve it again if it is not the case?

Copy link
Member

@TayouVR TayouVR left a comment

Choose a reason for hiding this comment

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

the develop branch had a crash in my testing, the PR didn't. hence I tried to clarify the comment.
But as stated I need to see what is actually happening.. when i have time (I have job interviews and other appointments every day until friday next week, so I have to see if I can squeeze some debugging in somewhere)

@Scrumplex Scrumplex merged commit 2b17a61 into PrismLauncher:develop Nov 23, 2023
32 checks passed
@Trial97 Trial97 added this to the 8.1 milestone Nov 23, 2023
@Scrumplex Scrumplex added the manual backport PRs that have been backported manually label Nov 28, 2023
LunaisLazier pushed a commit to LunaisLazier/ShatteredPrism that referenced this pull request Feb 9, 2024
Fully reload the instance list once the folder is changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog manual backport PRs that have been backported manually simple change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instance half changing
4 participants