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

release with multiple asi loader versions #119

Merged
merged 15 commits into from
Apr 9, 2024
Merged

release with multiple asi loader versions #119

merged 15 commits into from
Apr 9, 2024

Conversation

x87
Copy link

@x87 x87 commented Apr 7, 2024

closes #11

@x87 x87 marked this pull request as ready for review April 7, 2024 19:53
@x87 x87 requested a review from MiranDMC April 7, 2024 19:53
@x87
Copy link
Author

x87 commented Apr 7, 2024

/cc @CanerKaraca23

@x87
Copy link
Author

x87 commented Apr 7, 2024

@MiranDMC
Copy link
Collaborator

MiranDMC commented Apr 7, 2024

Readme file in both cases mentions ASI Loaded.
The "upd1" part in name "SA.CLEO_upd1+Silent_ASI_Loader.zip" will be different, right?

@x87
Copy link
Author

x87 commented Apr 8, 2024

Readme file in both cases mentions ASI Loaded.

what do you mean by "both cases"?

The "upd1" part in name "SA.CLEO_upd1+Silent_ASI_Loader.zip" will be different, right?

it will be the tag name under normal flow

@MiranDMC
Copy link
Collaborator

MiranDMC commented Apr 8, 2024

what do you mean by "both cases"?

Inside both release packs.
https://github.com/cleolibrary/CLEO5/blob/v5.0.0-alpha.54/README.md?plain=1#L22

@MiranDMC
Copy link
Collaborator

MiranDMC commented Apr 8, 2024

It should be 'SA.CLEO_xxx_SDK.zip' instead of SA.CLEO_xxx+SDK.zip

Maybe simple SA.CLEO_xxx.zip instead of SA.CLEO_xxx_no_ASI_Loader.zip

@CanerKaraca23
Copy link

CanerKaraca23 commented Apr 8, 2024

I think the sorting of download section should be like:

  • CLEO
  • CLEO + Silent's Asi Loader
  • CLEO + Ultimate Asi Loader
  • CLEO SDK (not "+" because it's only SDK)

And yes, i think "no asi loader" is unnecessary too.

Also i think it will be good adding "vorbisHooked.dll" for Ultimate Asi Loader, but it's optional.

@x87
Copy link
Author

x87 commented Apr 8, 2024

It will be _SDK https://github.com/cleolibrary/CLEO5/pull/119/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R117

I wanted _no loader so people don't assume this is the default choice. This archive is only for people who already have ASI loader. Otherwise I can imagine tons of questions like: "I downloaded CLEO.zip, why nothing happens when I run the game?"

@CanerKaraca23
Copy link

CanerKaraca23 commented Apr 8, 2024

It will be _SDK https://github.com/cleolibrary/CLEO5/pull/119/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R117

I wanted _no loader so people don't assume this is the default choice. This archive is only for people who already have ASI loader. Otherwise I can imagine tons of questions like: "I downloaded CLEO.zip, why nothing happens when I run the game?"

Yeah, you're right. Keeping it will be good for regular users. Also adding a huge title to release section like "You need an asi loader to run CLEO" on top of the changelog will be good because many people will not read readme and probably Google will redirect directly to CLEO's GitHub Releases. cleo.li website also need an update with modern look.

Edit: Didn't know cleo.li redirects GitHub, sorry.

@x87
Copy link
Author

x87 commented Apr 8, 2024

cleo.li website also need an update with modern look.

in progress

@MiranDMC
Copy link
Collaborator

MiranDMC commented Apr 8, 2024

Maybe there should be no release without ASI loader at all? Is there any reason why UAL should not be default option?

@x87
Copy link
Author

x87 commented Apr 8, 2024

I dunno. I personally never used UAL in San Andreas (mainly because Silent's ASI loader was distributed with CLEO for years). We just give people options and they can choose whatever works for them.

No loader should be present in my opinion for those people who have ASI loader already (not necessarily SAL/UAL), so they just want to unzip the archive and not deal with extra libraries.

@x87
Copy link
Author

x87 commented Apr 8, 2024

Also adding a huge title to release section like "You need an asi loader to run CLEO" on top of the changelog

added an example here, what do you think?

With these instructions I think we can remove _no_loader from the base archive.

@CanerKaraca23
Copy link

CanerKaraca23 commented Apr 8, 2024

Also adding a huge title to release section like "You need an asi loader to run CLEO" on top of the changelog

added an example here, what do you think?

With these instructions I think we can remove _no_loader from the base archive.

Looks good. You can also correct if it's "CLEO5" or "CLEO 5" and the "," on first sentence.

The XXX (upd1) for showing the version?

Also still think SDK should be last.

@x87
Copy link
Author

x87 commented Apr 8, 2024

Also still think SDK should be last.

it will be last when it's generated with _ in the name, not +

asset files are sorted by name

@CanerKaraca23
Copy link

Also still think SDK should be last.

it will be last when it's generated with _ in the name, not +

asset files are sorted by name

I see, thanks.

@x87
Copy link
Author

x87 commented Apr 8, 2024

Updated download instructions again, giving more clarity on each choice.

@MiranDMC
Copy link
Collaborator

MiranDMC commented Apr 8, 2024

Also adding a huge title to release section like "You need an asi loader to run CLEO" on top of the changelog

Does not matter how big text you place somewhere, there always will be (great?) percentage of regular users that will ignore it then complain. That's why "default" option should contain some kind of loader, be listed as first entry on list, and look like most basic, not complicated option to go.

@x87
Copy link
Author

x87 commented Apr 8, 2024

  • removed _no_loader from base archive name
  • added auto instructions

@x87 x87 requested a review from CanerKaraca23 April 8, 2024 12:31
@CanerKaraca23
Copy link

CanerKaraca23 commented Apr 8, 2024

README.md should be updated as well as @MiranDMC mentioned since UAL added too.

@CanerKaraca23
Copy link

Also adding a huge title to release section like "You need an asi loader to run CLEO" on top of the changelog

Does not matter how big text you place somewhere, there always will be (great?) percentage of regular users that will ignore it then complain. That's why "default" option should contain some kind of loader, be listed as first entry on list, and look like most basic, not complicated option to go.

Hmm. Removing No ASI Loader option will still cause regular users (ignoring the guide ones) ask about "what is the difference? which to install?, which one is better?" for asi loaders of download options.

Then there is two options, merge this PR, or maintain the status quo with SAL.

@CanerKaraca23
Copy link

I think also adding vorbisHooked.dll to UAL package will help people who want to uninstall asi loader or/and cleo. Also in UAL's GitHub readme.

@x87
Copy link
Author

x87 commented Apr 8, 2024

I think also adding vorbisHooked.dll to UAL package will help people who want to uninstall asi loader or/and cleo. Also in UAL's GitHub readme.

I don't think this is something CLEO should do. If the author of UAL does not provide a backup file, neither should we. You can open a ticket in UAL repo requesting the change.

@CanerKaraca23
Copy link

I think also adding vorbisHooked.dll to UAL package will help people who want to uninstall asi loader or/and cleo. Also in UAL's GitHub readme.

I don't think this is something CLEO should do. If the author of UAL does not provide a backup file, neither should we. You can open a ticket in UAL repo requesting the change.

Screenshot_20240408-191131_Chrome

@x87
Copy link
Author

x87 commented Apr 8, 2024

I think also adding vorbisHooked.dll to UAL package will help people who want to uninstall asi loader or/and cleo. Also in UAL's GitHub readme.

I don't think this is something CLEO should do. If the author of UAL does not provide a backup file, neither should we. You can open a ticket in UAL repo requesting the change.

Screenshot_20240408-191131_Chrome

OK, what does it mean for this PR?

@CanerKaraca23
Copy link

I think also adding vorbisHooked.dll to UAL package will help people who want to uninstall asi loader or/and cleo. Also in UAL's GitHub readme.

I don't think this is something CLEO should do. If the author of UAL does not provide a backup file, neither should we. You can open a ticket in UAL repo requesting the change.

Screenshot_20240408-191131_Chrome

OK, what does it mean for this PR?

Just mentioned as an optional. It's up to you of course.

@x87 x87 merged commit 3535844 into master Apr 9, 2024
1 check passed
@x87 x87 deleted the upd1 branch April 9, 2024 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple Release Archives
3 participants