-
Notifications
You must be signed in to change notification settings - Fork 69
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 support for transmitting the status's message to the C# code #2588
Conversation
…ames in the generator.
ksp_plugin/interface.hpp
Outdated
@@ -155,7 +156,8 @@ KeplerianElements ToKeplerianElements( | |||
QP ToQP(DegreesOfFreedom<World> const& dof); | |||
QP ToQP(RelativeDegreesOfFreedom<AliceSun> const& relative_dof); | |||
|
|||
Status ToStatus(base::Status const& status); | |||
Status const* ToStatus(base::Status const& status); | |||
Status const* ToStatus(base::Error error, std::string const& message); |
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.
Ownership of the result is given to the caller.
Since these are not interface functions directly, we should document this.
On the other hand, since these are not interface functions, we have a way of documenting this: return std::unique_ptr, and call ToStatus(status).release()
in the actual interface function.
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.
return std::unique_ptr
Hm, that does not work since Status is not RAII because of its C string.
Document that ownership is returned, and call it MakeStatus
or NewStatus
.
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.
ToNewStatus
.
tools/journal_proto_processor.cpp
Outdated
@@ -553,7 +553,7 @@ void JournalProtoProcessor::ProcessRequiredMessageField( | |||
} | |||
field_cxx_mode_fn_[descriptor] = | |||
[](std::string const& type) { | |||
return type + "*"; | |||
return type + " const*"; |
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.
Why? It feels odd to return ownership to the caller, but not mutability.
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.
Good point, done.
Changed the flight planner to output the error and message in case of unexpected error, and verified that the status is correctly propagated to the game.
As usual there is a commit before the change and a commit after the change for the generated files.
Fix #2583.
Contributes to #2585.