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

Add command-line options #90

Merged
merged 13 commits into from
Oct 6, 2024
Merged

Add command-line options #90

merged 13 commits into from
Oct 6, 2024

Conversation

saile515
Copy link
Contributor

This PR adds command-line options as outlined in #74

Closes #74

@lionkor
Copy link
Member

lionkor commented Jun 22, 2024

hey, what's the status on this? This looks good to me

@saile515
Copy link
Contributor Author

@lionkor As far as I know it is done. I made it a draft since I don't know the codebase too well, and didn't know if I implemented the feature in all relevant places. If you think it looks good, then it should be ready to merge.

lionkor added a commit that referenced this pull request Jun 28, 2024
these will be replaced by #90 eventually
@lionkor
Copy link
Member

lionkor commented Jun 28, 2024

Could we please also add a --dev option (same as no launch + no download + verbose), and a --no-update option so it wont update the launcher?

lionkor added a commit that referenced this pull request Jun 28, 2024
these will be replaced by #90 eventually
lionkor added a commit that referenced this pull request Aug 10, 2024
these will be replaced by #90 eventually
@lionkor lionkor marked this pull request as ready for review August 10, 2024 21:17
@WiserTixx WiserTixx changed the base branch from linux to master September 22, 2024 20:03
@WiserTixx WiserTixx requested a review from lionkor October 4, 2024 11:02
@lionkor
Copy link
Member

lionkor commented Oct 5, 2024

@WiserTixx let's remove the skip ssl verify again since we don't have it/dont need it in the curl branch, which is merged

Copy link
Member

@lionkor lionkor left a comment

Choose a reason for hiding this comment

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

Awesome, small changes needed but great thank you

src/Startup.cpp Outdated
std::string Arg;
for (int c = 2; c <= argc; c++) {
for (int c = 0; c <= options.game_arguments_length; c++) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? Shouldn't this pass the entire argc/argv, since we're relaunching the launcher, not the game, here? Same for the other Relaunch/URelaunch ones

@@ -183,26 +183,13 @@ void CheckForUpdates(int argc, char* args[], const std::string& CV) {
"&pk="
+ PublicKey + "&branch=" + Branch,
EP);
URelaunch(argc, args);
URelaunch();
#endif
} else
Copy link
Member

Choose a reason for hiding this comment

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

Let's please add a print when the no_update parameter is set, so that people know that their launcher is outdated but that the update was not applied

Comment on lines -193 to -205
void CustomPort(int argc, char* argv[]) {
if (argc > 1) {
std::string Port = argv[1];
if (Port.find_first_not_of("0123456789") == std::string::npos) {
if (std::stoi(Port) > 1000) {
DEFAULT_PORT = std::stoi(Port);
warn("Running on custom port : " + std::to_string(DEFAULT_PORT));
}
}
if (argc > 2)
Dev = true;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

the wicked witch is dead 🎉

@WiserTixx
Copy link
Collaborator

@lionkor I've fixed relauching and removed ssl verify. It's now also using the game_arguments when launching the game on windows. I have not implemented that for linux yet because I can't test it.

@lionkor
Copy link
Member

lionkor commented Oct 6, 2024

It's the same for linux I would assume, please implement it, i'll test it

@lionkor lionkor merged commit 259b215 into BeamMP:master Oct 6, 2024
2 checks 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.

add command-line options
3 participants