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

enabled GenericManagedPackPage for all non managed packs #1727

Closed
wants to merge 5 commits into from

Conversation

Trial97
Copy link
Member

@Trial97 Trial97 commented Oct 18, 2023

Parrent PR: #1405
should fix: #1394 (comment)
From that comment looks like I did not fix the original issue just extended the functionality of the managed packs(downloaded through prism from modrinth/curseforge)
This PR aims to have a generic managed pack page for all non-managed packs, allowing the user to update them using files.
How to test:

  • download a modrinth pack( an older version)
  • install it using the mrpack file
  • download a newer version of the pack(outside of prism)
  • click edit instance on the created instance
  • you should have a new tab named Pack Manager(I'm bad at names; feel free to suggest one)
  • entering that page you will be greeted with:
    • the pack name(the name of the instance)
    • a big button with upload from file
    • a changelog that reflects the notes content
  • click on the upload from the file
  • select the new version
  • profit

Also fixes #1731

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97 Trial97 added bug Something isn't working simple change changelog:merged A PR that will be merged into a parent PR in the changelog labels Oct 18, 2023
@Trial97 Trial97 added this to the 8.0 milestone Oct 18, 2023
@TheKodeToad
Copy link
Member

TheKodeToad commented Oct 18, 2023

Maybe have the tab be called Update, Updates, Updater, Update Manager (any of those) across mod platforms - perhaps just the icon could change
and the default icon could just be the updater icon

@TheKodeToad
Copy link
Member

It would nice to be able to bind and unbind from a provider too...

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97
Copy link
Member Author

Trial97 commented Oct 18, 2023

It would nice to be able to bind and unbind from a provider too...

Nice but let's keep this PR for just this(open an issue regarding this)

I updated the icon( now it should use the updater icon), and decided on "Pack Manager" for the page name, and that only if it is not curse or modrinth

@TheKodeToad
Copy link
Member

Maybe have the tab be called Update, Updates, Updater, Update Manager (any of those) across mod platforms - perhaps just the icon could change and the default icon could just be the updater icon

I think this is better tbh and i don't want to open another pr before 8.0

@Trial97
Copy link
Member Author

Trial97 commented Oct 18, 2023

are you having an issue with the new page name(Pack Manager) or the fact that is not the same name across providers?

@TheKodeToad
Copy link
Member

I can try implementing it myself...

@DioEgizio
Copy link
Member

Maybe have the tab be called Update, Updates, Updater, Update Manager (any of those) across mod platforms - perhaps just the icon could change and the default icon could just be the updater icon

What about just calling it "Modpack"?

@TheKodeToad
Copy link
Member

Yeah... I realised those look a bit weird

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@TheKodeToad
Copy link
Member

That will break the Website: link + you need to translate

@Trial97
Copy link
Member Author

Trial97 commented Oct 18, 2023

@TheKodeToad you either tell me exactly the lines or describe better. As I really do not understand what you want from me?
What will break the Website? what link?
The translation stuff is related to the return from here: https://github.com/PrismLauncher/PrismLauncher/pull/1727/files#diff-98ffe9e93f22873408db9da76d7f772d0cd74d88abe5106a26340f62ac4fb60dR145?

@TheKodeToad
Copy link
Member

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97
Copy link
Member Author

Trial97 commented Oct 18, 2023

Thanks that helps a lot.

@DioEgizio
Copy link
Member

it's adding the tab even on manual instances, but you can't update from onesix zips so this probably is a problem

@TheKodeToad
Copy link
Member

it's a bit weird

@Trial97
Copy link
Member Author

Trial97 commented Oct 19, 2023

Yeah, there is no way right now to update from onsix zip. And is not so simple to add it right now(it needs a bit of a rewrite on how onsix instance is created.

Should I remove both the PR and the issue from the milestone?
Maybe we can just add a warning, regarding this, and have it implemented in a following release?

@TheKodeToad
Copy link
Member

actually it should be fine as-is

@DioEgizio
Copy link
Member

I actually think this is a really big UX problem so unless we can exclude onesix now imo we should move this out of the milestone

@TheKodeToad
Copy link
Member

TheKodeToad commented Oct 19, 2023

The point is to include OneSix/non-managed though?

@DioEgizio
Copy link
Member

I mean the problem is that there's a modpack section even on onesix instances

@Trial97
Copy link
Member Author

Trial97 commented Oct 19, 2023

@DioEgizio have a look at: #1732 this should be an alternative to this PR

@Trial97 Trial97 removed this from the 8.0 milestone Oct 21, 2023
@Trial97
Copy link
Member Author

Trial97 commented Oct 22, 2023

Closing this in favor of:#1732

@Trial97 Trial97 closed this Oct 22, 2023
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:merged A PR that will be merged into a parent PR in the changelog simple change
Projects
None yet
3 participants