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

C++: Replace the many parameters of rerun::spawn with a struct #4142

Closed
kyle-figure opened this issue Nov 3, 2023 · 2 comments · Fixed by #4149
Closed

C++: Replace the many parameters of rerun::spawn with a struct #4142

kyle-figure opened this issue Nov 3, 2023 · 2 comments · Fixed by #4149
Labels
😤 annoying Something in the UI / SDK is annoying to use 🌊 C++ API C/C++ API specific 🏎️ Quick Issue Can be fixed in a few hours or less
Milestone

Comments

@kyle-figure
Copy link

Describe the annoyance
Overriding one default parameter in RecordingStream::spawn requires passing in all previous parameters:

// Spawn viewer with flush timeout of 10 seconds:
rec->spawn(9876, "75%", "rerun", std::nullopt, 10.0F).exit_on_failure();

Expected behavior
Be able to override a single default parameter without providing all previous default values.

Your goals
It would be nice to define the configs in a struct with defaults and pass it into the function that way. With this, we would be able to do:

// Spawn viewer with flush timeout of 10 seconds:
rec->spawn({.flush_timeout_sec=10.0}).exit_on_failure();

Desktop (please complete the following information):

  • OS: Ubuntu 22.04 x86_64
@kyle-figure kyle-figure added 👀 needs triage This issue needs to be triaged by the Rerun team 😤 annoying Something in the UI / SDK is annoying to use labels Nov 3, 2023
@Wumpf
Copy link
Member

Wumpf commented Nov 4, 2023

ah good point, a parameter struct might be nicer here indeed! We don't get to use the fancy C++20 (or C99...) struct assignment in our examples for backwards compatibility, but it's still a win!

@Wumpf Wumpf added 🏎️ Quick Issue Can be fixed in a few hours or less 🌊 C++ API C/C++ API specific and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Nov 4, 2023
@Wumpf Wumpf added this to the 0.11 C++ polish milestone Nov 4, 2023
@emilk emilk changed the title [CPP SDK] rerun::spawn() is annoying to override default values C++: Replace the many parameters of rerun::spawn with a struct Nov 6, 2023
@Wumpf
Copy link
Member

Wumpf commented Nov 6, 2023

#4149 ironically doesn't move the flush_timeout_sec into the struct because rerun::spawn doesn't take it. But everything else and and I added a chrono overload. So you can do now:

using namespace std::chrono_literals;
// ...
rec.spawn({}, 4s);

Wumpf added a commit that referenced this issue Nov 6, 2023
…am::spawn` with a `struct` (#4149)

### What

* Fixes  #4142

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4149) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4149)
- [Docs
preview](https://rerun.io/preview/3ff39f98e3391a563a02494cdb91a8eb8cfdcfcb/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/3ff39f98e3391a563a02494cdb91a8eb8cfdcfcb/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 🌊 C++ API C/C++ API specific 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
2 participants