Expose TTC computation to esminiJS API - #824
Conversation
7f72004 to
481cc0f
Compare
|
@johschmitz |
| return copy_state_from_object(object, scenario_engine_->getSimulationTime()); | ||
| } | ||
|
|
||
| double OpenScenario::get_ttc(int object_a_id, int object_b_id, bool free_space, int cs, int dist_type) const |
There was a problem hiding this comment.
For consistency with the rest of the codebase I suggest time_to_collision() since
I understand this is a wrapper for TimeToCollision()
There was a problem hiding this comment.
thanks, that is better.
Renamed to get_time_to_collision() keeping the get_ prefix to match the other esminiJS getters.
a3dfb0d
There was a problem hiding this comment.
Not 100% sure how consistent this is being handled in this project (maybe we need to cross check) but according to my most common understanding getters just access a value of a single object while this method is performing calculations across two object's data. Hence, IMHO for this particular method the get_ is somewhat misleading/confusing (unless some other code constantly calculates all TTCs towards all other objects and then stores a huge array of these values alongside every object and that is what we are accessing here).
There was a problem hiding this comment.
Thanks, agreed. Not a pure getter, so dropped get_.
a7caf64
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
|
|
||
| scenarioengine::Object* obj_a = scenario_engine_->entities_.GetObjectById(object_a_id); | ||
| scenarioengine::Object* obj_b = scenario_engine_->entities_.GetObjectById(object_b_id); | ||
| if (obj_a == nullptr || obj_b == nullptr) |
There was a problem hiding this comment.
Lots of return -1 extra checks. I think not all of these checks are needed, some of it is handled inside the wrapped method and maybe some conditions can never occur. IMHO add necessary checks to avoid crashes but other than that try to make the wrapper as thin as possible and if necessary add additional checks to the wrapped method so that WASM ans non-WASM builds behave as similar as possible.
There was a problem hiding this comment.
made it thinner and kept only the null checks needed to avoid a crash.
dc9b1a4
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
|
@johschmitz @eknabevcc |
|
LGTM |
|
LGTM too. |
Follow-up to #821, which added TTC to the esminiLib C API.
This adds the same
Object::TimeToCollisioncomputation to the esminiJS (WASM)API, so it can be called from the browser build via
get_ttc.