-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add initial support for Quilt server #1514
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
Conversation
itzg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Can you also add a test similar to tests/setuponlytests/pufferfish?
scripts/start-deployQuilt
Outdated
| QUILT_INSTALLER_VERSION=$(maven-metadata-release https://maven.quiltmc.org/repository/release/org/quiltmc/quilt-installer/maven-metadata.xml) | ||
| fi | ||
| export INSTALLER=quilt-installer-${QUILT_INSTALLER_VERSION}.jar | ||
| export SERVER=quilt-server-launch.jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to qualify the file name by the server version to enable in place upgrades. If the installer doesn't let you override that, then you could rename the jar to the versioned name.
scripts/start-deployQuilt
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| export FAMILY=QUILT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to use one of the existing family values unless it is indeed not compatible with any other. If that's the case then you'll need to double check any later conditions on FAMILY that might need to be extended.
|
I hope I added the test correctly, I more or less just took the one from Pufferfish and modified the docker-compose.yml and verify.sh. Hope I changed the right things. |
|
New changes look great. The PR verification should be giving it a test run now. |
tests/setuponlytests/quilt/verify.sh
Outdated
| @@ -0,0 +1 @@ | |||
| mc-image-helper assert fileExists "/data/quilt-server-launch-*.jar" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it should be
| mc-image-helper assert fileExists "/data/quilt-server-launch-*.jar" | |
| mc-image-helper assert fileExists "/data/quilt-server-*-launch.jar" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be fixed now
Exactly what it says on the tin, support for the Quilt server type.
The start script is basically just a heavily modified version of the one for fabric, just changing the way it is installed.
While Fabric provides an easy way to obtain the server jar directly, Quilt uses their own installer tool written in java, thus I download this tool and then execute it with the correct directories and versions set.
This should resolve #1484.