-
Notifications
You must be signed in to change notification settings - Fork 9
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: add 'proper' argument parsing #71
Conversation
#[cfg(not(target_arch = "wasm32"))] | ||
#[derive(Debug, Parser)] | ||
#[command(about = "bevy gaussian splatting render pipeline plugin", version, long_about = None)] | ||
struct CliArgs { |
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.
I think merging CliArgs
and GaussianSplattingViewer
structs would be great (can happen in a future PR though, let me know your bandwidth):
bevy_gaussian_splatting/viewer/viewer.rs
Line 47 in 0c28bac
pub struct GaussianSplattingViewer { |
e.g. this allows args like --height
, --width
, --hide_fps
, --hide_editor
, etc.
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.
makes sense
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.
as for bandwidth, I plan on working / contributing on this project long term. I'll play around with merging the structs and see how it goes.
#[cfg(not(target_arch = "wasm32"))] | ||
pub fn get_arg(n: usize) -> Option<String> { | ||
std::env::args().nth(n) | ||
pub fn get_arg(arg: MainArgs) -> Option<String> { |
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.
converting this function to:
pub fn get_args() -> CliArgs
(or pub fn get_args() -> GaussianSplattingViewer
with the above suggestion) would simplify argument retrieval and avoid duplicate parse calls.
here is a good reference: https://github.com/mosure/bevy_args |
use clap crate and enum for arg parsing.
usize
and thenusize
to back toString
seems unnecessary / redundant.Closes #26