Skip to content

Conversation

@NotAFile
Copy link
Contributor

@NotAFile NotAFile commented Dec 8, 2019

This allows you to get notified when enet is done with the packet, i.e. it has been transmitted successfully.

@aresch
Copy link
Owner

aresch commented Dec 8, 2019

Can you please add a test?

@NotAFile
Copy link
Contributor Author

NotAFile commented Dec 8, 2019

Actually, put this on hold, it's probably a good idea to pass the packet to the callback function. That way, it can use the ENET_PACKET_FLAG_SENT to find out if the packet has actually been sent successfully, which pyenet currently does not expose.

@aresch
Copy link
Owner

aresch commented Dec 8, 2019

What's the point of the userdata field? Is it just so you keep a python reference count?

Also, is it necessary to keep a function pointer for each packet? Why not just define one callback function on the host for on packet free?

@NotAFile
Copy link
Contributor Author

NotAFile commented Dec 8, 2019

What's the point of the userdata field?

The userData field is provided by enet (on a number of structs) and common to use with callback systems to allow "passing" data to the callback function. As we can not call python functions directly from C, the userData we pass is a pointer to the python callable we want to be called.

The way I did things here is inspired by cython's callback example: https://github.com/cython/cython/tree/master/Demos/callback

Is it just so you keep a python reference count?

The python object is converted into a void * and stored in the external enet struct, so cython does not have any way of knowing what's going on. That's why I had to add the Py_INCREF()/Py_DECREF code to manually tell python that I'm storing and then later deleting a pointer to the callable object.

Also, is it necessary to keep a function pointer for each packet? Why not just define one callback function on the host for on packet free?

freeCallback is defined on the ENetPacket struct in include/enet.h and can, to my knowledge, not be globally set.

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

Successfully merging this pull request may close these issues.

2 participants