Added a button to launch in Demo mode #903
Conversation
There was a problem hiding this comment.
looks good!
two things:
-
Unlike the normal launch and the offline launch options, the demo launch does not have a shortcut set (Like 'Ctrl+O' for the normal launch and 'Ctrl-Shift-O' for the offline one). While I don't know if the demo option will be used enough to warrant a shortcut for itself, it looks kinda odd comparing them side by side o.O
-
Seems like the 'demo' option isn't currently working for older MC versions. This is due to the legacy launcher not propagating the state. I've made a patch fixing this, I think it would fit in this PR instead of going into a separate one, what do you think?
the patch:
From 9fce42c01c20338891033e802e5de12195f3cbc8 Mon Sep 17 00:00:00 2001
From: flow <flowlnlnln@gmail.com>
Date: Wed, 13 Jul 2022 15:45:41 -0300
Subject: [PATCH] fix: allow demo for older versions
We were not propagating the '--demo' flag in the legacy launcher,
unconditionally setting the 'demo' parameter to false.
Signed-off-by: flow <flowlnlnln@gmail.com>
---
libraries/launcher/org/polymc/applet/LegacyFrame.java | 5 +++--
libraries/launcher/org/polymc/impl/OneSixLauncher.java | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/libraries/launcher/org/polymc/applet/LegacyFrame.java b/libraries/launcher/org/polymc/applet/LegacyFrame.java
index 2cdd17d7..7ae56e60 100644
--- a/libraries/launcher/org/polymc/applet/LegacyFrame.java
+++ b/libraries/launcher/org/polymc/applet/LegacyFrame.java
@@ -63,7 +63,8 @@ public final class LegacyFrame extends Frame {
int winSizeH,
boolean maximize,
String serverAddress,
- String serverPort
+ String serverPort,
+ boolean isDemo
) {
// Implements support for launching in to multiplayer on classic servers using a mpticket
// file generated by an external program and stored in the instance's root folder.
@@ -106,7 +107,7 @@ public final class LegacyFrame extends Frame {
appletWrap.setParameter("sessionid", session);
appletWrap.setParameter("stand-alone", "true"); // Show the quit button.
appletWrap.setParameter("haspaid", "true"); // Some old versions need this for world saves to work.
- appletWrap.setParameter("demo", "false");
+ appletWrap.setParameter("demo", isDemo ? "true" : "false");
appletWrap.setParameter("fullscreen", "false");
add(appletWrap);
diff --git a/libraries/launcher/org/polymc/impl/OneSixLauncher.java b/libraries/launcher/org/polymc/impl/OneSixLauncher.java
index 362ff8d6..28c3aaa6 100644
--- a/libraries/launcher/org/polymc/impl/OneSixLauncher.java
+++ b/libraries/launcher/org/polymc/impl/OneSixLauncher.java
@@ -137,7 +137,8 @@ public final class OneSixLauncher implements Launcher {
winSizeH,
maximize,
serverAddress,
- serverPort
+ serverPort,
+ mcParams.contains("--demo")
);
return;
--
2.35.1
I was thinking of using 'Ctrl-Alt-O' or 'Ctrl-Shift-D'.
I have applied the patch to this pr. As I agree that they fit in together. |
I guess the 'Ctrl+Alt+O' makes more sense to me (the one with 'D' looks more like a delete operation IMO xd), but I don't know about it triggering the menu bar, it's quite odd :/ Anyway, i discovered that Ctrl+D is for copying instances, which is quite an odd choice xD so i guess you can do whatever you want! |
After setting a shortcut for 'Ctrl+Alt+O' it no longer inherits the action of 'Ctrl+Alt' |
great! :D |
|
This doesn't work if the instance you are trying to start hasn't downloaded all libraries yet. Apart from that, I don't really like that the button is as prominent as the other two launch options. The vast majority of users will probably never start demo mode. Maybe it would be better to make it a checkbox or something when clicking on |
I've noticed this too, but it seems to be a general problem with the offline mode, not just the demo, so idk if it's in this PR's scope o.O |
|
Demo mode was added in Release 1.3, would be a good idea to disable versions before 1.3 |
Signed-off-by: jopejoe1 <johannes@joens.email>
We were not propagating the '--demo' flag in the legacy launcher, unconditionally setting the 'demo' parameter to false. Signed-off-by: flow <flowlnlnln@gmail.com>
Signed-off-by: jopejoe1 <johannes@joens.email>
Signed-off-by: flow <flowlnlnln@gmail.com>
Signed-off-by: flow <flowlnlnln@gmail.com>
e.g. >= 1.3.1 instances Signed-off-by: flow <flowlnlnln@gmail.com>
|
@jopejoe1 i've rebased and made a couple of changes with respect to the comments above. Can you take a look and see if you agree with those? 🥺 I'm not sure whether we should leave the demo button in the instance settings page, while it is less important than the other two, we also have a lot of empty space in there, so it's not a bit issue either. What do you think? |
|
The changes you made look good to me. 🙂 Edit: With moving the Launch Demo button into the Launch Offline button, it is not possible to launch demo mode together with a profiler. |
yeah, but I don't see much value in that tbh. what would be the use case of profiling demo mode? xD |
true 🙃 |
Adding a button to launch in demo mode, similar to the button that launches offline mode.
Supersedes #899
Closes #867