Skip to content

Array acknowledge #23

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

Closed
jianjunz opened this issue Jun 8, 2015 · 8 comments
Closed

Array acknowledge #23

jianjunz opened this issue Jun 8, 2015 · 8 comments

Comments

@jianjunz
Copy link
Contributor

jianjunz commented Jun 8, 2015

If an ack is an array, socket.io-client only returns the second element of the ack (sio_socket.cpp: line 399-412). Is there any plan to add support for array ack?

@melode11
Copy link
Contributor

melode11 commented Jun 8, 2015

Yes, the first element will be the event name, I haven't figure out what's the most preferred organization of the rest part. Do you have any suggestions?

@jianjunz
Copy link
Contributor Author

Align C++ client's behavior with JavaScript client's?

@stefanhong
Copy link

I second LittleLittle. As shown in http://socket.io/docs/, client's ack handler should receive the same list of arguments as sent by the server. It took me awhile to realise that C++ client skip the first element.

@jianjunz
Copy link
Contributor Author

The first element is event name. I think it's the reason to be skipped.

The JavaScript client allows variable arguments for callback function. So it can receive the same list sent by the server.

I took some time trying to allow variable arguments for callback function by using new message::list data structure. But it may add one more public API.
void emit(std::string const& name, message::list const& msglist = nullptr, std::function<void (message::list const&)> const& ack = nullptr)

How do you think about this change and what's the expected behavior if the developer still using the old API message::ptr but server sends a list of argument?

Currently, I just delete line 399-412 of sio_socket.cpp and test the ack if it is an array or a string in my app.

@melode11
Copy link
Contributor

@stefanhong @littlelittle Fully agree that the best way is to keep the behavior the same with JavaScript did. Seems I need to do some breaking changes.

@stefanhong
Copy link

Maybe we can just pass message::flag_array back to ack handler directly instead of unpack it? I just encountered another related issue in that server return fn(err, data) back to ack handler, where err might be undefined when there is no error. C++ client will crash because it tries to unpack using array_ptr->get_vector()[0]->get_flag(), but array[0] is nullptr.

@melode11
Copy link
Contributor

Pushed tag 1.5.0 to support this.

@jianjunz
Copy link
Contributor Author

Thanks for your effort to make it better :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants