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

Allow joining servers to work on 1.20 #1164

Closed
wants to merge 3 commits into from

Conversation

TheKodeToad
Copy link
Member

@TheKodeToad TheKodeToad commented Jun 12, 2023

I swear I'm not copying everything arthomnix does

Suggested commit on meta:

From: TheKodeToad <TheKodeToad@proton.me>
Date: Mon, 12 Jun 2023 12:15:22 +0100
Subject: [PATCH] Automatically add quickPlay trait

Signed-off-by: TheKodeToad <TheKodeToad@proton.me>
---
 meta/model/mojang.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/model/mojang.py b/meta/model/mojang.py
index 2e35634..969ed0d 100644
--- a/meta/model/mojang.py
+++ b/meta/model/mojang.py
@@ -19,6 +19,7 @@ SUPPORTED_LAUNCHER_VERSION = 21
 SUPPORTED_COMPLIANCE_LEVEL = 1
 DEFAULT_JAVA_MAJOR = 8  # By default, we should recommend Java 8 if we don't know better
 COMPATIBLE_JAVA_MAPPINGS = {16: [17]}
+QUICK_PLAY_START = datetime.fromisoformat("2023-04-05T12:05:17+00:00")
 
 """
 Mojang index files look like this:
@@ -247,6 +248,11 @@ class MojangVersion(MetaBase):
         else:
             raise Exception(f"Unsupported compliance level {self.compliance_level}")
 
+        if self.release_time >= QUICK_PLAY_START:
+            if not addn_traits:
+                addn_traits = []
+            addn_traits.append("quickPlay")
+
         major = DEFAULT_JAVA_MAJOR
         if (
             self.javaVersion is not None
-- 
2.37.2

I can open another PR if it's better

Signed-off-by: TheKodeToad <TheKodeToad@proton.me>
@TheKodeToad TheKodeToad changed the title Basic Quick Play support Allow joining servers to work on 1.20 Jun 12, 2023
@Scrumplex
Copy link
Member

Scrumplex commented Jun 12, 2023

about that meta commit:

maybe try to detect the presence of these instead
https://github.com/PrismLauncher/meta-upstream/blob/5558b713b05250b4a9822c2d6ea9ef72f0ff1251/mojang/versions/1.20.json#L53-L108

@TheKodeToad
Copy link
Member Author

TheKodeToad commented Jun 12, 2023

i did consider that, seems a bit complex though and maybe actually less safe
oh, thought it's worth mentioning: the reason i opted for a quickPlay trait as opposed to legacyServerJoin is the latter would break a lot of old versions if you used customise

@getchoo getchoo added bug Something isn't working simple change labels Jun 12, 2023
@TheKodeToad TheKodeToad added this to the 7.1 milestone Jun 12, 2023
@DioEgizio
Copy link
Member

Meta commit when

@Ryex
Copy link
Contributor

Ryex commented Jun 16, 2023

I think the meta commit shouldvset the trait based off the presence of the rules not a timestamp. But otherwise this looks good

@TheKodeToad TheKodeToad removed this from the 7.1 milestone Jun 16, 2023
@TheKodeToad
Copy link
Member Author

I removed it from the milestone as I think we really need to get 7.1 out.

@DioEgizio DioEgizio added this to the 7.2 milestone Jun 18, 2023
@Scrumplex Scrumplex removed this from the 7.2 milestone Jul 5, 2023
@Scrumplex
Copy link
Member

I think long term, we should work on adopting Mojang's minecraftArgs data structure instead of throwing it away in meta and doing workarounds like this in Prism

@TheKodeToad
Copy link
Member Author

that sounds like a good idea! considering joining servers is broken isn't this good enough for now though?

this would require changing the format of components :/ not a bad thing, but would be non-trivial

@TheKodeToad TheKodeToad added the changelog:fixed A PR that appears under "Fixed" in the changelog label Jul 30, 2023
Signed-off-by: TheKodeToad <TheKodeToad@proton.me>
@Trial97 Trial97 mentioned this pull request Oct 1, 2023
1 task
@DioEgizio
Copy link
Member

is this pr just dead?

@TheKodeToad
Copy link
Member Author

Can we just merge it because it works and clearly nothing else has been done since?

Signed-off-by: TheKodeToad <TheKodeToad@proton.me>
@Trial97
Copy link
Member

Trial97 commented Nov 22, 2023

@TheKodeToad can you have a look at my meta PR: PrismLauncher/meta#31 it implements what you described but instead uses the game arguments

@TheKodeToad
Copy link
Member Author

@Scrumplex is this what you were talking about?

I thought you meant to change the launcher-side implementation to be able to recognise the official arg format.

@Scrumplex
Copy link
Member

Yeah I meant changing the meta format (perhaps reengineering it from scratch) so that we just support Mojang's arg format like most other launchers

@Trial97
Copy link
Member

Trial97 commented Nov 22, 2023

Can we go with that patch until we rewrite the meta in rust?

@j5155
Copy link

j5155 commented Feb 28, 2024

can this be merged, please? would love if Prism supported this feature at all, even if it's not the most elegant way
it could easily be rewritten later but that sounds like much more work/time, if this could be merged in the meantime it would be great

@n-aspen
Copy link

n-aspen commented Feb 28, 2024

I could very much use this feature at the moment too, and was actually surprised when I found out that it was documented but not working.

@Enchoseon
Copy link

For personal use, I duct-taped a hard-coded version of this commit on Gentoo Linux by placing this diff:

diff --git a/launcher/minecraft/MinecraftInstance.cpp b/launcher/minecraft/MinecraftInstance.cpp
index 7cb4100..e752615 100644
--- a/launcher/minecraft/MinecraftInstance.cpp
+++ b/launcher/minecraft/MinecraftInstance.cpp
@@ -658,8 +658,7 @@ QStringList MinecraftInstance::processMinecraftArgs(AuthSessionPtr session, Mine
     }
 
     if (serverToJoin && !serverToJoin->address.isEmpty()) {
-        args_pattern += " --server " + serverToJoin->address;
-        args_pattern += " --port " + QString::number(serverToJoin->port);
+	args_pattern += " --quickPlayMultiplayer " + serverToJoin->address + ':' + QString::number(serverToJoin->port); 
     }
 
     QMap<QString, QString> token_mapping;
diff --git a/libraries/launcher/org/prismlauncher/launcher/impl/StandardLauncher.java b/libraries/launcher/org/prismlauncher/launcher/impl/StandardLauncher.java
index 1869091..f5335bc 100644
--- a/libraries/launcher/org/prismlauncher/launcher/impl/StandardLauncher.java
+++ b/libraries/launcher/org/prismlauncher/launcher/impl/StandardLauncher.java
@@ -76,10 +76,8 @@ public final class StandardLauncher extends AbstractLauncher {
         }
 
         if (serverAddress != null) {
-            gameArgs.add("--server");
-            gameArgs.add(serverAddress);
-            gameArgs.add("--port");
-            gameArgs.add(serverPort);
+	    gameArgs.add("--quickPlayMultiplayer");
+	    gameArgs.add(serverAddress + ':' + serverPort); 
         }
 
         // find and invoke the main method

—into /etc/portage/patches/games-action/prismlauncher/forceQuickPlayMultiplayer.patch and re-emerging Prismlauncher, which did the trick and allowed me to not have to deal with the meta package.

Prior to this, when I tried to pass the --quickPlayMultiplayer argument in various ways, Prismlauncher or Java would (1) remove it, or (2) crash for being an unrecognized argument, respectively.

I'm 99% sure I just missed an option somewhere in the launcher, but the ability to pass arguments direct might be another possible solution to this problem if the maintainer doesn't want to hack something into the meta package so that one could do something like this:

$ prismlauncher --launch "1.20.4 Creative FAWE" -x "--quickPlayMultiplayer='127.0.0.1:31337'"`

@n-aspen
Copy link

n-aspen commented Mar 1, 2024

I was able to find this comment on a related issue that has fixed it for me without having to compile a custom version. It's a bit hacky with you having to edit the json for the minecraft version, but it at least worked for what I'm doing.
#975 (comment)

@Trial97 Trial97 added this to the 8.3 milestone Mar 26, 2024
@timoreo22 timoreo22 added the backport release-8.x Backport PR automatically label Mar 26, 2024
@Trial97
Copy link
Member

Trial97 commented Mar 26, 2024

Anying as is I made a meta PR with just the patch that Kode mentioned:PrismLauncher/meta#38
for testing use the following meta URL: https://raw.githubusercontent.com/Trial97/prism-meta-launcher/join_servers
please also delete the meta folder inside the root directory and create a new instance with the server option set

@Trial97
Copy link
Member

Trial97 commented Mar 27, 2024

closing this in favor of: #1164 as the meta PR was merged for that one

@Trial97 Trial97 closed this Mar 27, 2024
@TheKodeToad TheKodeToad removed this from the 8.3 milestone Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-8.x Backport PR automatically bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog simple change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants