chr15m / PodSixNet

Lightweight multiplayer network library for python games
https://pypi.python.org/pypi/PodSixNet
GNU Lesser General Public License v3.0
167 stars 32 forks source link

Replacement for Serializable #23

Open ROTARTSI82 opened 6 years ago

ROTARTSI82 commented 6 years ago

I realized in the new 0.10.0 update PodSixNet.rencode.serializable was removed. I am wondering if there is a replacement because all of my code breaks on 0.10.0 so I'm sticking to the old version.

chr15m commented 6 years ago

@ROTARTSI82 yes I'm sorry about that, I removed the serializable() stuff as it didn't seem possible to support both python2 and python3 using that hack.

It might be possible to get it working again, maybe by using a Python pickle. I'll try to find some time to investigate.

In the mean time probably the best strategy would be to serialize to pure Python data-structures before sending objects over the network.

chr15m commented 6 years ago

@ROTARTSI82 if you can provide a simple example of the way you are using the old serializable() call then I'll see if I can hack support back in and add unit tests for that.

ROTARTSI82 commented 6 years ago

I'm using serializable to send instances of pygame sprites over a connection. I think I can just send the arguments for __init__() and __name__ over the network instead.

ROTARTSI82 commented 6 years ago

I also found this version of PodSixNet for python3.4 that supports serializable:

pod-six-net-python3.4 by tborisova

chr15m commented 6 years ago

@ROTARTSI82 yes this looks like the py3k version which @selfsame made on the python3 branch in this repository.

Did you test it, and does it actually support serializable? When I was merging the python3 branch I found that the unit tests for the serializable part did not actually pass, which was part of the reason why I deprecated it.

Another reason was I had trouble getting a version to work when some clients are python2 and some clients are python3. I'd be willing to get this merged back in if we can get it working in such a way that the tests pass and there is interoperability between python2 clients and python3 clients.

The other alternative is we could throw a deprecation exception when serializable is used and advise people how to upgrade their code. In general I don't like breaking backwards compatibility in software but that serializable() thing always seemed like a hack and a possible security issue to me. I'd prefer pure data structures to be the only thing to go over the wire in PodSixNet.

Let me know what you think.

selfsame commented 6 years ago

Ah sorry back when I ported rencode.py I never got the class serialization tests to pass

https://github.com/chr15m/PodSixNet/blob/python3/PodSixNet/rencode.py#L584

fix would involve rolling back to that version and figuring out what wasn't working

Incidentally, did we depreciate podsixnet for python 2?

chr15m commented 6 years ago

@selfsame all good, thanks for contributing the python 3 code, it was super useful.

PodSixNet still works on python 2. I managed to reconcile the two branches and they even interoperate (python 2 clients on python 3 server and visa versa).

I think the way forward is to see if there is a simpler implementation for the class serialization code which will work for both python 2 and python 3 and is secure (e.g. not pickling). Could do something like serializing __dict__ only. If we can get that working and tests pass I'll merge it.

chr15m commented 6 years ago

In the short term I should probably throw a deprecation exception if the serializable() call is made by client code with a note we're trying to get it working again and workarounds.

pauliyobo commented 5 years ago

wouldn't using pickle solve the issue?

chr15m commented 5 years ago

@pauliyobo no, the problem here is not the serialization method but the absence of an old interface after some refactoring.

In any case, pickle is not a great way to serialize objects over the network as it suffers a number of limitations outlined in the documentation, including a security issue.

pauliyobo commented 5 years ago

Ah i understand