FlowArg / FlowUpdater

The free and open source solution to update Minecraft.
https://flowarg.github.io/FlowUpdater
GNU General Public License v3.0
97 stars 19 forks source link

Incorrect link generated when an OptiFine version (preview) is provided without the preview_ prefix #16

Closed LangArthur closed 1 year ago

LangArthur commented 1 year ago

Hi flow ! I may have found a bug for preview version of optifine. Describe the bug When I try to download the last preview of optifine (OptiFine HD U I2 pre5), I get a FlowUpdaterException. I am award that it might be my fault since the optifine version is filled in a string.

To Reproduce Steps to reproduce the behavior:

Expected behavior No exception and a flawless download.

Desktop (please complete the following information):

Additional context I had a look at your OptiFineIntegration code, and to me, the jar name is good, so it shouldn't be a name problem

            final String name = preview ?
                    (optiFineVersion.contains("preview_") && optiFineVersion.contains("OptiFine_") ?
                            optiFineVersion + ".jar" :
                            "preview_OptiFine_" + optiFineVersion + ".jar") :
                    "OptiFine_" + optiFineVersion + ".jar";

Also, does it not make more sense to detect if a version is a preview in the OptiFineInfo(name) constructor than having a boolean parameter that you can miss ? Or does the preview version name of optifine vary that much ?

antoineok commented 1 year ago

Hi, can you send the exception message please?

LangArthur commented 1 year ago
[07:19:50] [FlowUpdater] [ERROR]: An error occurred : fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg,
<!-- Optifine - 728x90 Dynamic (610159a627b9ac245369f0e7) - 728x90, 970x250, 970x90 - Place in <BODY> of page where ad should appear -->,<div c,                     
<!-- Optifine - 728x90 Dynamic (610159a627b9ac245369f0e7) - 728x90, 970x250, 970x90 - Place in <BODY> of page where ad should appear -->, <div c,
<!-- Optifine - Rich Media (6101597e8b95466018aad5c2) - 1x1 - Place in <BODY> of page where ad should appear -->, <div class="vm-placement" data-id="6101597e8b9]
</html>/table>an>ef="contact">Contact</a></a>ifine" target="_blank"><img alt="Reddit" src="images/reddit.png"></a>></a>ent -->
   at fr.flowarg.flowupdater.integrations.optifineintegration.OptiFineIntegration.getOptiFine(OptiFineIntegration.java:58)
   at fr.flowarg.flowupdater.integrations.IntegrationManager.loadOptiFineIntegration(IntegrationManager.java:186)
   at fr.flowarg.flowupdater.FlowUpdater.loadModLoader(FlowUpdater.java:175)
   at fr.flowarg.flowupdater.FlowUpdater.updateMinecraft(FlowUpdater.java:142)
   at fr.flowarg.flowupdater.FlowUpdater.update(FlowUpdater.java:126)
   [... all the call related to my project ]

edit: I removed some spaces and add backlines returns to make message more readable

antoineok commented 1 year ago

wait, can you post the complete unedited error message

LangArthur commented 1 year ago

Sure, there it is :

[04:37:15] [FlowUpdater] [ERROR]: An error occurred : fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg,                     <!-- Optifine - 728x90 Dynamic (610159a627b9ac245369f0e7) - 728x90, 970x250, 970x90 - Place in <BODY> of page where ad should appear -->, <div c,                     <!-- Optifine - 728x90 Dynamic (610159a627b9ac245369f0e7) - 728x90, 970x250, 970x90 - Place in <BODY> of page where ad should appear -->, <div c,     <!-- Optifine - Rich Media (6101597e8b95466018aad5c2) - 1x1 - Place in <BODY> of page where ad should appear -->, <div class="vm-placement" data-id="6101597e8b9] </html>/table>an>ef="contact">Contact</a></a>ifine" target="_blank"><img alt="Reddit" src="images/reddit.png"></a>></a>ent -->
   at fr.flowarg.flowupdater.integrations.optifineintegration.OptiFineIntegration.getOptiFine(OptiFineIntegration.java:58)
   at fr.flowarg.flowupdater.integrations.IntegrationManager.loadOptiFineIntegration(IntegrationManager.java:186)
   at fr.flowarg.flowupdater.FlowUpdater.loadModLoader(FlowUpdater.java:175)
   at fr.flowarg.flowupdater.FlowUpdater.updateMinecraft(FlowUpdater.java:142)
   at fr.flowarg.flowupdater.FlowUpdater.update(FlowUpdater.java:126)
antoineok commented 1 year ago

[04:37:15] [FlowUpdater] [ERROR]: An error occurred : fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg.flowupdater.utils.FlowUpdaterException: fr.flowarg,

this seems incomplete

LangArthur commented 1 year ago

I agreed, this output was weird so I did more investigation. For some reasons, my IDE cut the error message, I still don't know why but anyways. The good news is I found the problem and a potential fix.

As you can see at the following link, the link to download the optifine jar does not contain "preview" inside, so the getJsonPreview function was not able to found a suitable link.

The easy fix is to remove "preview" in the following line (OptiFineIntegration.java, getJsonPreview method, l.113) :

            final Optional<String> result = Arrays.stream(respLine).filter(s -> s.contains("downloadx?f=preview")).findFirst();

As I understood, you implemented this, so I'm asking you if it seems an acceptable fix. If it is, I'll submit a PR tomorrow with the fix and maybe a test with optifine integrations (depending on my available time)

FlowArg commented 1 year ago

This modification is not suitable because it would break the entire system. The good fix is to provide to the function the correct argument. I will do this in the next release as well. In the meantime, just add previewOptiFine at the beginning of your version and everything will be fine

LangArthur commented 1 year ago

Ohhh sorry, I understand now. I though the link has changed format for all preview version, but the format I was testing was in fact wrong (this link exist but the download fail). So yes, I'll try to tweak the link, to get what I want.

FlowArg commented 1 year ago

Issue fixed (tested with 1.20.1_HD_U_I6_pre3 because preview versions for 1.19.3 are not available anymore)