-
Notifications
You must be signed in to change notification settings - Fork 7
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
Low level APIs #10
Comments
I fully agree with this proposal :) The compat API should just be that - compatibility. All the efforts should go into a nice, polished API, see #5. |
Definitely agree here. Having been through the existing codebase I think we can probably take its concepts and rebuild from there, providing a porcelain wrapper over the top (most likely with clever use of an I think our main effort for the low level API needs to start with the socket handling. Right now it basically errors our when you get a problem, including if the socket is closed for some reason. I want this to be hidden away from the high level API and do as much error handling, reconnecting, etc to hide the use of sockets from the end user. |
Great. I think a compatible API that works in Python 3 is important, and an extended version that's more Pythonic and more feature rich would be ace. Low level & high level sounds great too. I think abstraction is good but don't want to rely on it. Functions like def build_house(x, y, z, width, height, block):
"""
Builds a house at location (x, y, z) size width x height made of block type block
"""
... This might just make a hollow cube with walls and a roof, with a door at the front. The code would be simple enough that the user could understand it and make their own function that did something else. Thought occurs: is it more pythonic to pass in the location as a 3-tuple? Or Vec3? |
I don't think we should worry too much about Create_wall, Create_house, any position in the game world should be passed as vector3. Not sure On 29. september 2014 12:05:10, Ben Nuttall wrote:
|
Functions like that shouldn't be the goto for main usage, just quick tools for getting started and demonstrating. |
It would be great to have these examples available for examples/teaching so maybe we should include them in their own package? Example:
|
+1 On 29. september 2014 12:25:44, George Hickman wrote:
|
They could even serve as examples in the documentation for users/teachers where we show how to build up a |
Sounds good 👍 |
I'd just keep the examples in the docs, or in a Regarding the vectors, I already commented on this in #14, I'd actually pass in flat variables (i.e. three arguments called If you still want to use vectors, rename them to something more sensible like If, on the other hand, you always work with integers and floats, that is easy to understand for a beginner. If have a function called I know that this doesn't look beautiful to a software engineer. But always assume that you'll have to explain this API to a child that has never programmed before :) (I actually will next month, so that's why I'm pushing for an API that's as simple as possible.) |
@dbrgn – There are two things here, samples being quickly available to users/teachers and code location. These samples are going to be very useful to have on hand when teaching and it would be nice for users to be able to import working code (ie that we've tested). This is all about lowering the barrier to entry for the library and having to copy/paste code from somewhere that may or may not work isn't going to help this goal. We shouldn't assume the repo/GitHub will be the main distribution point for the codebase given it is likely going to be packaged for the Raspbian repos too. You've made some good points about the use of vectors vs arguments but could we keep that in #14 or start a new issue so things don't get too muddled please. |
+1 for keeping "samples" in the code, albeit skeletal samples as per @bennuttall |
OK, I agree about tested samples. If they're called |
@dbrgn – Yes, that's a good point. |
Samples 👍 x, y, z 👍 |
This issue is Low level APIs and there was much support for my proposal. It has now become a discussion of where to put sample files. As a result, I don't know what the design decision tag is supposed to refer to. Please could we all be more careful about this - you can always open a new issue if you wish. |
Added new issue #40 for discussion of location of samples |
OK, so here is my proposal regarding the low level pythonic API. I think it should stay very close to the Minecraft Pi API, while at the same time being clean and easy to use. But without too many abstractions, so we should prefer working with basic data types instead of creating our own. That's something that can go into the higher level API. ImportsI think the low level API should reside in a subpackage. A few suggestions for possible names:
All important objects / types / functions should be importable directly from that package. from minecraft.core import Minecraft ConnectingThere's a single class that can be instantiated. It should immediately connect to the server and keep the connection alive. The low level connection should be available at mc = Minecraft()
another_mc = Minecraft(host='192.168.1.10') There should be no global state at all. Everything should be accessible via that instance. NamespacingI'd suggest a simple system of namespaces, without having to instantiate any additional things.
Function definitionsFor the low level API, I'd go with flat arguments, with no position type. IIRC there was a lot of support for this in other discussions. The block types should just be integers (or alternatively enums). Coordinates are integers or floats (player position). world
chat
player
camera
ExceptionsWe should have a sensible system of exceptions, but keep them minimal. Connection related issues should be solved separately in the connection class if possible (e.g. automatic reconnecting). If all fails, then we can throw an exception. Right now the following possibilities come to my mind:
Sample implementationHere's a basic implementation of parts of the suggested API: https://github.com/coredump-ch/pyminecraft/blob/master/pyminecraft/minecraft.py It could be used as a starting point. The connection class is based on the mcpi one. It could still be improved and extended. |
@dbrgn – This is great! I'm really happy with the API you've laid out here. One thing I'd suggest is switching |
Absolutely :) I updated the post above. |
So, any suggestions regarding the naming of the package? Some suggestions:
I don't really like any of them, but think I'd go with core. As soon as this is sorted out I can do a pull request. |
If I had to pick any of those it'd be core |
+1 to core here. |
@dbrgn Your code is an improvement, in that it checks the number of arguments. But there's more to be done. |
@jonathanfine – While I agree a fresh start might be nice, I don't see any benefit for us here. The legacy code isn't completely terrible and while the connection class isn't perfect it's a good start that can be improved upon. As you said, @dbrgn's code is a step in the right direction so I would favour continuing in that direction and improving the connection class. |
I think the closer the low level API is to the TCP protocol, the better. It should be simple and stable. Not too many abstractions. |
Regarding the connection module - absolutely, it needs to be improved. This is mostly a copy from mcpi, to get the codebase to work. It's not something I designed or redesigned. |
I've written something that does the right things for me. See #48 Please review lowlevel API code for URL. See #44 Minecraft Low Level API: First draft for review for @dbrgn's version. @dbrgn and others. |
I think we need two low-level APIs.
The only changes I'd like to make with the legacy API are those that support compatibility with legacy code. I think 2to3 has already made all those changes.
I'd like the replacement low-level API to be
I've done some experiments, and it seems that sending bad API requests to Minecraft can cause to freeze. We don't want that to happen, and we want to be able to reproduce this fault when it does.
So I'd also like the replacement API to provide
Logging will also help with testing and debugging the high-level API.
The text was updated successfully, but these errors were encountered: