-
Notifications
You must be signed in to change notification settings - Fork 70
Get rid of the dummies, and notice a few incorrect things #1229
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
Get rid of the dummies, and notice a few incorrect things #1229
Conversation
Edited to |
ksp_plugin/plugin.cpp
Outdated
PartId part_id, | ||
GUID const& vessel_guid, | ||
RelativeDegreesOfFreedom<AliceSun> const& from_parent) { | ||
not_null<Vessel*> vessel = find_vessel_by_guid_or_die(vessel_guid).get(); |
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.
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.
Done.
ksp_plugin/plugin.cpp
Outdated
part_id, | ||
1 * Kilogram, | ||
vessel->parent()->current_degrees_of_freedom(current_time_) + relative); | ||
not_null<Part*> part = vessel->part(part_id); |
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.
const
ksp_plugin/plugin.cpp
Outdated
PlanetariumRotation().Inverse()(from_parent); | ||
ephemeris_->Prolong(current_time_); | ||
AddPart( | ||
vessel, |
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.
The indent is weird, I'd prefer to move this to the previous line and split the last parameter.
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.
Done.
ksp_plugin/plugin.cpp
Outdated
std::map<PartId, not_null<Vessel*>>::iterator it; | ||
bool emplaced; | ||
std::tie(it, emplaced) = part_id_to_vessel_.emplace(part_id, vessel); | ||
CHECK(emplaced); |
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.
Print the part_id and the vessel guid.
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 don't have the vessel guid.
ksp_plugin/plugin.cpp
Outdated
bool emplaced; | ||
std::tie(it, emplaced) = part_id_to_vessel_.emplace(part_id, vessel); | ||
CHECK(emplaced); | ||
auto deletion_callback = [ it, &map = part_id_to_vessel_ ] { |
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.
No space before/after []
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.
Done; clang-format does that when capturing an expression by reference somehow O_o
ksp_plugin/plugin.cpp
Outdated
map.erase(it); | ||
}; | ||
auto part = | ||
make_not_null_unique<Part>(part_id, |
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.
Would fit on the previous line.
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.
Done.
ksp_plugin/plugin.hpp
Outdated
// For a KSP |Vessel| |v|, the arguments correspond to |v.id|, | ||
// |v.orbit.referenceBody.flightGlobalsIndex|, |v.loaded|. | ||
virtual void InsertOrKeepVessel(GUID const& vessel_guid, | ||
Index parent_index, | ||
bool loaded, | ||
bool& inserted); | ||
|
||
// Adds a part with the given part_id to the vessel with the given ID, which |
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.
|part_id|
ID->guid
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.
Done.
ksp_plugin/plugin.hpp
Outdated
@@ -427,8 +429,15 @@ class Plugin { | |||
physics::KeplerianElements<Barycentric>> const& keplerian_elements, | |||
MassiveBody const& body); | |||
|
|||
void AddPart(not_null<Vessel*> vessel, |
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.
Comment: Adds a part to a vessel, recording it in the appropriate map and setting up a deletion callback.
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.
Done.
ksp_plugin/vessel.hpp
Outdated
// still parts left after the removals; thus a call to |AddParts| must occur | ||
// before |FreeParts| is first called. | ||
// since the last call to |FreePart|. Checks that there are still parts left | ||
// after the removals; thus a call to |AddParts| must occur before |FreeParts| |
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.
AddPart (no s)
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.
Done.
serialization/journal.proto
Outdated
@@ -774,6 +774,19 @@ message InsertOrKeepLoadedPart { | |||
optional In in = 1; | |||
} | |||
|
|||
message InsertUnloadedPart { | |||
extend Method { | |||
optional InsertUnloadedPart extension = 5024; |
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.
Invest in a new extension # to make sure that we don't confuse ourselves.
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.
Done.
PileUp
serialization should be nicer now.