Add packets #9
Draft
Add packets #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This rename is necessary as it prepares the file to be used for other things than just abstract classes. These other uses should be present in the same file, and will include major changes to the file, this commit just prepares for that.
The writer/reader classes were introducing a lot of repetition, which was very likely to grow as more types would get added (read/write byte, read/write short, read/write int, read/write long, ... and all that with both signed and unsigned versions). These functions were explicitly defined only because it was slightly more convenient and explicit type-wise, but this wasn't a good solution for long-term. Instead, this replaces explicit functions with a single function taking in an enum value defining what struct type should be read/written. To still provide type-checking, this new function includes overloads with Unions of literal enum values common for ints/floats/etc. allowing easy typing without too much repetition. Because struct is doing most of our serailization/deserialization, it has it's own error handling, which we catch and raise a ValueError with the same message for consistency. This allows us to avoid things like checking, meaning the whole enforce_decorator was removed. This also changes varnums and renames write/read_varnum to write/read_varuint, along with changing max size to use bits, which is more explicit, and makes things slightly easier for us. The regular write/read_varint is now also not using a static 32-bit (8-byte) size as regular ints, but rather an arbitrarily sized number depending on passed max_bits. Since this is a signed function which doesn't use a fixed amount of bits, this also adds to_twos_complement and from_twos_complement helper functions. This is essentially almost a full rewrite of the base_io file, and while this is a little less convenient solution (instead of `writer.read_byte()` we now need `writer.read_value(StructFormat.BYTE)`, I think it's worth it for how much repetition it avoids.
These classes define a standardized interface across all classes which should support being read (deserialized) or written (serialized) with any BaseWriter/BaseReader.
While it may be slightly odd to use `bytes` instead of `bytearray` to be passed into a function for writing bytearrays, however it actually makes a lot of sense. Type-wise, `bytearray` is actually a subtype of bytes, so bytearrays can still be passed with the annotation being `bytes` type. The reason using `bytes` makes more sense is because this type also supports `__len__`, and can be sent over as an array of individual bytes. There is therefore no reason to disallow this type, and it does allow us to avoid needlessly casting/converting bytes to bytearrays.
The Packet class was originally using __init_subclass__, which is called each time a child class is created. In there, we were performing a check that ensures that this child class does define PACKET_ID class variable. This does works, however it is against the nature of ABCs, since it doesn't allow the class to be extendable through another abstract class, it forces all subclasses to already have PACKET_ID, which is needed for concrete subclasses, but not necessarily for abstract subclasses, hence preventing more abstract subclasses to be made from this class. Instead of that, this commit replaces __init_subclass__ with simple check in __init__, as every subclass is expected to call the __init__ of it's super class, triggering this PACKET_ID check with it. Since __init__ only runs on initialization (making of an instance), not on making of another class itself, it behaves much more like python's ABCs are expected to, and allows extendability by other abstract classes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for sending messages over several different packets, allowing us to send arbitrary information other than just messages in these packet classes, able to read/write (receive/send) themeselves.