[Batch - PART1] Handle control update event#81
Conversation
Add method handlers and update the scenes cache when receiving an onControlUpdate event
|
|
||
| int cache_new_control(interactive_session_internal& session, const char* sceneId, interactive_control& control, rapidjson::Value& controlJson) | ||
| { | ||
| if (session.controls.find(control.id) != session.controls.end()) |
There was a problem hiding this comment.
You'll want to lock the scenesMutex before calling find on controls or scenes.
std::shared_lock<std::shared_mutex> l(session->scenesMutex);
| { | ||
| std::string scenePointer = "/" + std::string(RPC_PARAM_SCENES) + "/" + std::to_string(sceneIndex++); | ||
| auto thisSceneId = scene[RPC_SCENE_ID].GetString(); | ||
| if (nullptr != sceneId) |
There was a problem hiding this comment.
Is this nullptr check because SCENE_ID wouldn't exist for some reason? If that's at all possible you should actually check if the property exists first using HasMember.
There was a problem hiding this comment.
Sorry - confusing names. sceneId is an argument to a function, thisSceneId is a local variable within the for loop.
Purpose is that you can optionally specify a specific scene to update the control pointers for instead of blindly updating them for all scenes.
There was a problem hiding this comment.
Also with regards to the HasMember check it depends how defensive we want to be - in other places in the code we're trusting these fields to be there (and according to the current protocol they should) so I don't check that the member exists here because this value only gets updated with an object from the socket
There was a problem hiding this comment.
Ah right yeah I misread that.
Great, if the protocol should have the field we don't need an extra check.
| { | ||
| std::string scenePointer = "/" + std::string(RPC_PARAM_SCENES) + "/" + std::to_string(sceneIndex++); | ||
| auto thisSceneId = scene[RPC_SCENE_ID].GetString(); | ||
| if (nullptr != sceneId) |
There was a problem hiding this comment.
Ah right yeah I misread that.
Great, if the protocol should have the field we don't need an extra check.
|
|
||
| int cache_new_control(interactive_session_internal& session, const char* sceneId, interactive_control& control, rapidjson::Value& controlJson) | ||
| { | ||
| if (session.controls.find(control.id) != session.controls.end()) |
2 participants
Add method handlers and update the local scenes cache when receiving an
onControlUpdateevent.Stacked PR for batch updates.