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

feat: Launch arguments #301

Open
wants to merge 61 commits into
base: main
Choose a base branch
from

Conversation

Alystrasz
Copy link
Contributor

@Alystrasz Alystrasz commented Apr 30, 2023

This interface lists launch arguments (stolen from wiki page), and enables users to toggle them. A description of each argument can be displayed by hovering related item.

This is merely an interface with the ns_startup_args.txt file.

launch_args_selector.webm

Closes #147.

TODOs
  • -language argument
  • Should we let users write anything they want as launch args? (I don't think so)
  • Can users set up envvars the same way as launch args?
  • i18n
  • Handle duplicated arguments in file
  • Distinction between "official" and "custom" arguments
  • Fix clippy issues
  • Remove tag animations

@Alystrasz
Copy link
Contributor Author

@GeckoEidechse maybe you'll have answers to some of my questions 😃

@GeckoEidechse
Copy link
Member

Holy fuck this looks really cool :O

Should we let users write anything they want as launch args? (I don't think so)

I'm tending towards a yes though in that case it should then also display the raw string in a text box somewhere ^^

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Probably a bit too early to do a review atm but I was in the mood ^^"

In regards to clippy errors, half of them are caused by "unused function" which makes no sense to me cause clearly the function is used in the macro to set up tauri. Maybe one of the Rust wizards (cat, emerald, snagg, ...) in the Northstar Discord have an idea. For the rest feel free to poke me if you need/want any help <3

src-tauri/src/main.rs Outdated Show resolved Hide resolved
src-vue/src/components/LaunchArgumentsSelector.vue Outdated Show resolved Hide resolved
src-tauri/src/northstar/launch_arguments.rs Outdated Show resolved Hide resolved
src-vue/src/components/LaunchArgumentsSelector.vue Outdated Show resolved Hide resolved
@Alystrasz
Copy link
Contributor Author

While FlightCore UI can try preventing users from breaking their launch arguments configuration by adding random words in it, I feel like the rust backend should be responsible for its integrity.
What do you think should happen when backend detects a malformed arguments file? Overwrite it with correctly-formatted arguments? Refuse to parse it and raise an error?

@GeckoEidechse
Copy link
Member

What do you think should happen when backend detects a malformed arguments file? Overwrite it with correctly-formatted arguments? Refuse to parse it and raise an error?

Correcting user mistake sounds very difficult to do tbh. It makes more sense IMO to just reject with error and if possible say what's wrong (though the second part of that might also be quite difficult to do ^^)

@ASpoonPlaysGames
Copy link
Contributor

Honestly, there shouldn't rly be any handling of this sorta thing. It's almost impossible to know which launch args exist and are valid and stuff. Launch args are for advanced users and they should not need their hand to be held

@ASpoonPlaysGames
Copy link
Contributor

To elaborate:

  • Any convar (which mods can add) is a valid launch arg (with the + prefix i believe)
  • Plugins can just check command line args for whatever they want, and there isn't a manifest or anything for it
  • Launcher can do the same as plugins (but we can control that a bit more)

@Alystrasz
Copy link
Contributor Author

Thanks for your feedback!
What will be the correct way to use launch arguments once associated file has been removed (R2Northstar/NorthstarLauncher#517)? Directly as executable arguments?

@F1F7Y
Copy link

F1F7Y commented Jul 27, 2023

Directly as executable arguments?

Exactly that

To expand on @ASpoonPlaysGames comment:

Personally i think the user should have access to the ability to set commandline arguments in a text entry ( just like steam does it). You could then add what you show off in the initial PR comment into the DEV section with useful arguments to make testing easier.

@github-actions github-actions bot added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Dec 20, 2023
@Alystrasz
Copy link
Contributor Author

Alystrasz commented Jan 9, 2024

Since we're talking removing the ns_startup_args.txt file again, I'll rework this by storing arguments in flightcore itself.

@github-actions github-actions bot removed the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Jan 12, 2024
@Alystrasz Alystrasz marked this pull request as ready for review January 28, 2024 17:30
@Alystrasz
Copy link
Contributor Author

Alystrasz commented Jan 28, 2024

image

I need help on the two following points:

  • making sure launch args are correctly sent to Steam
  • finding out why launch args is interpreted as a string by launcher (fixed in 2a4fc2f)

@github-actions github-actions bot added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflicts Blocked by merge conflicts, waiting on the author to resolve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Users cannot specify or edit launch args using FlightCore
5 participants