Skip to content

Conversation

JaiChoccyFox
Copy link
Contributor

Since LaunchWrapper is able to launch servers, it seems the current way it handles passing arguments to the main method doesn't account for old server versions which expect nogui as the first argument if the user wants to launch the server without GUI. This pull request adds a really small (and honestly silly) workaround for this case.

There is likely a much better way of handling arguments passed through for servers but this'll work for now I guess.

@Lassebq
Copy link
Member

Lassebq commented Mar 12, 2025

I feel like a better solution would be to preserve original order of arguments and for any arguments added by wrapper (such as UUID extracted from username or accessToken from UUID) to be appended to the end of argument list.

@JaiChoccyFox
Copy link
Contributor Author

Alright, I've done up a better solution than before. This should preserve the order of arguments the user sets, and the wrapper appends its parameters at the end (if said parameters haven't been set by a launcher or user). I've also added --noParameters as a wrapper-only parameter which prevents the wrapper from appending all its parameters at the end to prevent some server versions from complaining about unknown arguments.

Let me know if there's any changes you think should be made, or if there's probably another solution that would be better.

@JaiChoccyFox JaiChoccyFox changed the title Add silly workaround for the 'nogui' parameter. Slightly rework how the wrapper handles arguments/parameters. Mar 13, 2025
Instead of relying on 'parameters' & 'extraParameters' to determine what arguments to pass through to a client/server, use an array list that can store strings and parameter objects. This 'arguments' list is used to preserve the order of arguments that has been passed to the wrapper.

An additional parameter also gets added: 'noParameters'. This prevents the wrapper from appending its parameters to the arguments list which is useful for servers.
Copy link
Member

@Lassebq Lassebq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to merge. I'd just change .equals(Boolean.TRUE) since it's an instance of LaunchParameterSwitch

This prevented the wrapper from appending parameters into arguments, however, the likely chance of someone needing to use this is close to 0. Unless someone really wanted to do this, it's pretty much unnecessary.
@JaiChoccyFox
Copy link
Contributor Author

Alright, pushed a commit to remove --noParameters. I think that should be all if everything else is good.

@JaiChoccyFox JaiChoccyFox requested a review from Lassebq March 15, 2025 08:27
@Lassebq Lassebq merged commit 37c933f into MCPHackers:main Mar 15, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants