add fabric api message reopened #38
Conversation
# Conflicts: # src/main/java/net/fabricmc/installer/Handler.java
|
Why not just reopen the old PR? |
|
can't, I've deleted the repo. |
There was a problem hiding this comment.
This does not slove the issue, this is a cheap workaround.
-
A lot of users dont need fabric api
-
Those that do should not need to worry about installing it
-
Does not slove the issue for other launchers
-
Makes the installer specific to fabric where as fabric has always had the idea that someone else could make their own version without any limitations. fabric is just a normal mod.
-
I have plans to create an offline installer soon, this wont work well at all there.
-
It doesnt and cannot (without a lot of work) link to the correct version to install.
-
It too "in your face TBH" the /use page (where people get this installer from in the first place) has a discrete message telling them about fabric.
|
crab
This PR doesn't force anyone to install it, it's just a reminder for users to install fapi, nothing more. More people than not will use fapi, I know this is true because fapi is the most downloaded fabric-only mod on CF. And this can work offline too, it doesn't download the fapi for you. You don't have to provide the correct version either, the concept of a non-integrated api is somewhat new for most users, just reminding them that this is a fabric thing is enough I don't see how 1 extra popup is that "in your face" |
|
Solving the problem for MultiMC is out of scope for the installer. |
More users need fabric api and the people who don't need fabric api is the exception not the rule.
The people that don't need fabric API should know whether they need it.
Again this is a temporary fix for people that don't know well of fabric or modding in general, vanilla launcher should cover most cases
Go remove fabric-api from example mod and stop branding it as such, people need it and will continue to need it even if you remove it from example mod.
When will your plan come true? we could simply disable the message if it is offline.
You simply don't need to, link to the cf page as usual.
What. |
This message does not force those users to install it. It simply reminds the users that need it, to install it.
They don't, they can ignore the message
Other launchers are not our responsibility. Fabric installer is, and I bet most people still use fabric installer.
This does not make the installer specific to fabric api, it only recommends the installation of another mod, which the user can ignore, hence the user can use the installer even if they don't want fabric. It only makes the installer aware of fabric api, which shouldn't be an issue, especially for a temporary solution.
Define "won't work". The message will not pop up or something?
It can link to the curseforge page.
It clearly needs to be in your face as too many people are not installing it. Those that do not need it will either know they don't already, ignore the message or get it when they don't need it. Think about it, if a user gets it when they don't need it, that's not a problem at all. |
There is no real harm to installing it when it is not needed. Furthermore the qualification "most mods" still acknowledges that it is not mandatory.
I think this is a larger problem that can't be easily solved this way. This is indeed a temporary fix, but temporary fixes are better than no fixes at all.
Fabric API is a normal mod, but its mod ID of
That doesn't preclude just providing a message saying that Fabric API exists.
That is true, but this solves the problem of users not knowing that Fabric API exists and is separate from Fabric Loader. The problem of linking to the correct version is separate, more complex, probably better for another PR.
The message kind of needs to be "in your face," otherwise it will be ignored! |
|
Modmuss, you haven't proposed anything, we would rather hope you changed a bit shedaniel. I am sure shedaniel would gladly just change the popup to just be changing the installer screen. |
It's a precedent in many installers to have a popup/dedicated page when installed. This isn't new ground. |
|
The horizontal space in the screen is not quite big enough, will try and give a picture |
|
Also this kind of debate is exactly bikeshedding and, as pointed out on that website, is only harmful to overall user and developer experience. |
|
Maybe it is a modpack author's job to set up loader and api and mods correctly for players. Their pack distribution should handle api possibly, like handling other mods. |
|
If you only care about modpacks, you don't care about fabric installer. |
|
this is for regular users just to install a few mods manually. |
|
Modpacks use Curse or MultiMC |
|
So what about other mods? If I have charbone mobs that depends on charbone core, how does charbone mobs ask people to grab charbone core? How mods ask to grab fabric api should be done in a similar way. |
|
Charbone Mobs would JIJ Charbone Core because it isn't used by almost every mod. |
|
this message could be removed if mods start to jij fabric API, but until then, add this message. |
There was a problem hiding this comment.
I am genuinely trying to understand the opposing argument here, but none of those arguments really counter this temporary solution. If you do have a genuine counter-argument, after having read mine and others' messages on here, please let me know and I will remove my approval.
|
Locked before this get's review bombed. I think the next thing to do is get the other core fabric dev's input on this. |
There was a problem hiding this comment.
Right:
I've had a bit of time to think about this, and think it will do for now. Its not an ideal solution but being relaisitic the ideal solution that I talked about isnt going to be coming soon.
This change can easily be removed or changed in the future as its not an api or anything that is set in stone.
| @@ -30,5 +30,7 @@ prompt.server.jar.valid=Valid {0} server jar found | |||
| prompt.server.jar.invalid=No valid {0} server jar found | |||
| prompt.server.generate=Generate | |||
| prompt.server.overwrite=Are you sure you want to override the existing launch scripts? | |||
| prompt.install.successful.title=Successfully Installed | |||
| prompt.install.successful=Fabric Loader {0} for {1} has been successfully installed.<br>Most mods will also require you to install <a href="{2}">Fabric API</a>. | |||
There was a problem hiding this comment.
I think this message could be improved. Something to clarify that Fabric API is just a mod, and doesn't have an installer or anything.
Perhaps:
| prompt.install.successful=Fabric Loader {0} for {1} has been successfully installed.<br>Most mods will also require you to install <a href="{2}">Fabric API</a>. | |
| prompt.install.successful=Fabric Loader {0} for {1} has been successfully installed.<br>Many mods also depend on the <a href="{2}">Fabric API</a> mod which you can download and put in the mods folder in your instance. |
There was a problem hiding this comment.
Yes, it should mention to put it into the mods folder. Something along the lines of what @Prospector said put possibly removing the "in your instance."
There was a problem hiding this comment.
Changed to "Many mods also require you to download Fabric API mod into the mods folder."
There was a problem hiding this comment.
hmm, should have the in front of Fabric API mod but also I'm not sure the phrasing on "download ... into the mods folder" is very great. I'd just use what I put above but without "in your instance".
There was a problem hiding this comment.
Many mods also depend on the Fabric API which you can download and put in the mods folder is really long tbh.
how about this?
Many mods also require you to put the Fabric API mod into the mods folder.
There was a problem hiding this comment.
I don't believe the installer creates a mods folder, it only appears on first launch. You can expect that mentioning a nonexisting mods folder would be confusing to some people.
If we are going to mention the mods folder I would suggest having the installer create the mods folder, and adding a button in the installer to open it. That's a bit out of scope for this PR but I think it would be a useful improvement in the future.
There was a problem hiding this comment.
Humm, that is a good point. The installer could possibly create the mods folder when creating the profile, but then this would only work when the profile is creted (I assume most people leave this enabled).
I dont think this is a blocker for merging this PR however
There was a problem hiding this comment.
tbf people should know what a mods folder is, that's the minimum amount of knowledge to get the game modded.
There was a problem hiding this comment.
Updated the text to Many mods also require you to put Fabric API into the mods folder.
There was a problem hiding this comment.
One more alternative, just to throw it out there:
Most mods also require the Fabric API mod to be installed.
Why mention the mods folder and make it longer?
EDIT: I just noticed the change request below, ignore me.
| @@ -31,6 +31,6 @@ prompt.server.jar.invalid=No valid {0} server jar found | |||
| prompt.server.generate=Generate | |||
| prompt.server.overwrite=Are you sure you want to override the existing launch scripts? | |||
| prompt.install.successful.title=Successfully Installed | |||
| prompt.install.successful=Fabric Loader {0} for {1} has been successfully installed.<br>Many mods also require you to download <a href="{2}">Fabric API</a> mod into the mods folder. | |||
| prompt.install.successful=Fabric Loader {0} for {1} has been successfully installed.<br>Many mods also require you to put <a href="{2}">Fabric API</a> into the mods folder. | |||
There was a problem hiding this comment.
Imo mentioning mods folder is a bad idea. Users are supposed to know how to install mods; otherwise there is no point for them to install the api via this installer.
If fabric loader is installed via some bundled installation package like for custom client package that users don't need to put mods into the mods folder, the distributor should have their custom installer that calls this installer via cli/java, which would not have this prompt at all.
In conclusion, whenever the prompt can appear, the user is supposed to know how to add a mod (or give them the wiki link). Calling it "put into the mods folder" is a mess up in logical dependencies.
There was a problem hiding this comment.
Imo we might link to how to install a mod on the wiki, i.e. https://fabricmc.net/wiki/tutorial:adding_mods
There was a problem hiding this comment.
IMO we shouldn't care about users that are using modpacks, this installer is only for the vanilla launcher, in which there are no easy way to install a modpack, people who are using the vanilla launcher to install a modpack should know what they are doing since better options are usually offered such as multimc and twitch.
People who are installing via this installer are mostly the people that are installing mods themselves, which it is not bad to mention about the mods folder.
The wiki page sounds nice but I don't know how much that can help, you should be able to install mods into the mods folder as a user.
Also that wiki page is full of text and doesn't really differentiate itself from just "please add to mods folder"
There was a problem hiding this comment.
This installer is only used for installing into the mods folder. If automated methods are using this then something is prossibly wrong. (the meta api makes it trivial to re-impliment the installer).
Linking to a wiki page might not be a bad idea, or improving the curse page. (If someone wants to help with this let me know as writing things isnt my kind of thing)
There was a problem hiding this comment.
Imo we should keep the term "installing", and add a <a href="https://fabricmc.net/wiki/tutorial:adding_mods">installing</a> so people who don't know how to install a mod can go to that link and learn that putting the mod jar in the right folder is the way to go.
There was a problem hiding this comment.
In response to @liach 's initial:
Yeah, I understand your point. It is entirely reasonable to expect the user to know how to install a mod. At the same time, I've had to deal with users (at multiple points) who have required step-by-step instructions on how to locate their minecraft installation, including a note that it is not on their desktop, and that they should not try to execute the minelittlepony-mod jar file.
So I think the first step is to assume nothing. The user is a brick.
There was a problem hiding this comment.
Also would it help if at the end the installer had a button that just opens the mods folder for them?
There was a problem hiding this comment.
Both as a convenience, and an aid to the bricks out there.
There was a problem hiding this comment.
It is not viable to show the mods folder unless if create profile is checked, since this installer is just installing fabric loader into launcher versions.
…d server (#2) * fix color and show error text be4 message (FabricMC#34) * Update gradle + deps * No longer set the version `type` to release. * Bump version * Add Fabric API message (FabricMC#38) * add fabric api message * some changes * Change error pane, error if can't open desktop * Fix formatting * Remove unnecessary change * change to String.format and edit the text. * Update message * fix jar not being published This doesnt seem right but works * Fix some issues with parsing arguments * Revert "No longer set the version `type` to release." This reverts commit e365e57 * Write server JAR to a temporary file first (FabricMC#49) * Complete Fr translation (FabricMC#47) * Complete Fr Language * Fix Translation * Create installer_et_EE.properties (FabricMC#45) * Add files via upload (FabricMC#40) * Update installer_zh_CN.properties (FabricMC#39) * Update installer_zh_CN.properties * Update src/main/resources/lang/installer_zh_CN.properties Co-authored-by: liach <7806504+liach@users.noreply.github.com> * Update src/main/resources/lang/installer_zh_CN.properties Co-authored-by: liach <7806504+liach@users.noreply.github.com> Co-authored-by: liach <7806504+liach@users.noreply.github.com> * lang fix and add (FabricMC#42) * Installer 0.7 * Code cleanup * Move away from gson libary, reducing the download size a lot. * New native bootstrap for windows written in rust. * Fix signed build * Fix published artifact name. * Fix inverted check when validating the cli path when installing the client. I broke this in the refactor * Default the cli client install dir to the default minecraft dir. * 0.7.2 * Update bootstrap to resolve issue with missing vcruntime on windows * Improve exception displaying * Russian translation (FabricMC#68) * Move server library cache to target directory (FabricMC#70) * Migrate to Path, minor tweaks * Move server library cache to target directory * Update native bootstrap Update deps * fix the hard-coded manifest (FabricMC#76) * [SPON-1] fix hard-coded main * [SPON-1] fix variable naming * Unattended server installer (FabricMC#78) * Unattended server installer * Review feedback and fixes * Fix formatting of new files, will add checkstyle to the whole project next. * Validate server jar hash * Cleanup * checkstyle * add pt_BR (FabricMC#80) Co-authored-by: Maneschy <77683876+maneschy-d@users.noreply.github.com> * Update native bootstrap. Update gradle + plugins * Add option to select a custom loader jar, strip library signatures for server launcher jar * Show a warning when the vanilla launcher is open. (FabricMC#72) MacOS and Windows only for now. * Fix install button being disabled after an install failure. * Bump version * Create installer_fi_FI.properties (FabricMC#83) * Allow translating the vanilla server Downloading message (FabricMC#85) * Fall back to snapshot versions if no stable versions are available (FabricMC#86) * should hopefully be good? (FabricMC#82) * should hopefully be good? * should hopefully be all good * Add Spanish locale (FabricMC#84) * Create installer_es_ES.properties * Fix typo and wrong word choosing * Add download progress message to Finnish translation (FabricMC#87) * Exclude natives and images from server bootstrap jar. * Bump version * Update windows bootstrap. * Revert bootstrap update. * Xbox GamePass/Microsoft store support. (FabricMC#90) * Xbox GamePass/Microsoft store support. * Update bootstrap and bump version. * Don't install if the message box is closed. * Update installer_et_EE.properties (FabricMC#91) * Update Portuguese (Brazil) (FabricMC#92) Co-authored-by: Percario <77683876+Percario@users.noreply.github.com> * Fix launcher selection showing when no profile json files were found. * yes * show loader versions per minecraft versions * got server installer working * make client working * Bump version * revert changes * more fix * loader blacklist for 1.8.9 * fix style * Add support for installing a Fabric server without shading the libs into the launcher jar * Use a more reasonable MC server launch command line * Bring ServerLauncher in line with the regular installer (same lib dir, version check code, option to use local loader jar) * Some fix and improvement Co-authored-by: shedaniel <daniel@shedaniel.me> Co-authored-by: modmuss50 <modmuss50@gmail.com> Co-authored-by: comp500 <comp500@users.noreply.github.com> Co-authored-by: Mysterious_Dev <40738104+Mysterious-Dev@users.noreply.github.com> Co-authored-by: Madis Otenurm <Madis0@users.noreply.github.com> Co-authored-by: DarkKnightComes <darkknightcomes@gmail.com> Co-authored-by: Enaium <32991121+Enaium@users.noreply.github.com> Co-authored-by: liach <7806504+liach@users.noreply.github.com> Co-authored-by: kokkiemouse <39451248+kokkiemouse@users.noreply.github.com> Co-authored-by: Player <player@player.to> Co-authored-by: PinkGoosik <56874042+PinkGoosik@users.noreply.github.com> Co-authored-by: Player <sfPlayer1@users.noreply.github.com> Co-authored-by: Logic <38597904+LogicFan@users.noreply.github.com> Co-authored-by: Percario <77683876+Percario@users.noreply.github.com> Co-authored-by: Maneschy <77683876+maneschy-d@users.noreply.github.com> Co-authored-by: Juuxel <6596629+Juuxel@users.noreply.github.com> Co-authored-by: altrisi <altrisi.trillosierra@gmail.com> Co-authored-by: Foksha! <55259471+FokshaWasTaken@users.noreply.github.com>
this is #35
