py3minepi / py3minepi-legacy

Original attempt to port Minecraft Pi to Python 3
https://github.com/py3minepi/py3minepi
Other
25 stars 7 forks source link

Pythonic API #5

Closed dbrgn closed 9 years ago

dbrgn commented 9 years ago

I think a pythonic API is very important for this project. The code by Mojang is messy and full of Java idioms.

After bringing compatibility with Python 3 and adding tests, I think the first thing that should be done is adding a more pythonic API. The old function names could then be mapped to the new ones, so that they still work. The use should be discouraged though. In a future version they could even be deprecated with proper warnings.

I already started developing an alternative (Python 3 only) library for Minecraft Pi a few weeks ago. Here is the current API: https://github.com/coredump-ch/pyminecraft/blob/master/pyminecraft/minecraft.py Usage example:

from pyminecraft import Minecraft

mc = Minecraft()
mc.chat.post('Hello minecrafters!')

def jump(height):
    x, y, z = mc.player.get_tile()
    mc.player.set_tile(x, y, z + height)

Now that this project exists and is endorsed by the Raspberry Pi foundation, it doesn't make much sense to continue developing it. Maybe you can use the code as inspiration? I'd be happy to help with the development.

(BTW: I also copied the protocol docs that I started in the pyminecraft project to your wiki: https://github.com/py3minepi/py3minepi/wiki/Minecraft-Pi-Protocol-Specification)

jvlomax commented 9 years ago

We have actually started making things more pythonic already, and it looks similar to what you have :) I believe we decided that we should keep the old api available (though hidden), so that it doesn't break any existing code using the current (old) API

On 25. september 2014 17:11:30, Danilo Bargen wrote:

I think a pythonic API is very important for this project. The code by Mojang is messy and full of Java idioms.

After bringing compatibility with Python 3 and adding tests, I think the first thing that should be done is adding a more pythonic API. The old function names could then be mapped to the new ones, so that they still work. The use should be discouraged though. In a future version they could even be deprecated with proper warnings.

I already started developing an alternative (Python 3 only) library for Minecraft Pi a few weeks ago. Here is the current API: https://github.com/coredump-ch/pyminecraft/blob/master/pyminecraft/minecraft.py Usage example:

from pyminecraft import Minecraft mc = Minecraft() mc.chat.post('Hello minecrafters!')

def jump(height): x, y, z = mc.player.get_tile() mc.player.set_tile(x, y, z + height)

Now that this project exists and is endorsed by the Raspberry Pi foundation, it doesn't make much sense to continue developing it. Maybe you can use the code as inspiration? I'd be happy to help with the development.

— Reply to this email directly or view it on GitHub https://github.com/py3minepi/py3minepi/issues/5.

ghickman commented 9 years ago

@dbrgn – Thanks for putting the time into making a pythonic port! We'd love to have your help on this project.

A new API (which is very similar to yours) was designed at the PyConUK sprints, unfortunately it doesn't seem to have been added to the Wiki yet.

@jvlomax @bennuttall @jonathanfine – Were any notes made for the proposed API?

jonathanfine commented 9 years ago

I have some sample files on my laptop - not to hand at the moment. I also committed at the sprint some working code, presently in minecraft/space.py, which provides I think a better API.

I've moved on a bit since then, and now favour something like:

box = Box(pos1, pos2)
for pos in box:
    pos.block = SAND

if pos in box:
   pass

if box1 in box2:
   pass

big_box = box1 + box2

vec = pos2 - pos1
box3 = box1 + vec

I'm strongly influenced by mathematical thinking, and so my ideas may need correction from practice. It would be good to take some existing code and rewrite it to be more Pythonic. That will provide an very helpful force in the design of the API.

It would be great if we could gather sample code that we'd like to re-implement in the new API. Here's a starter

pozorvlak commented 9 years ago

I really like the idea of using set union and intersection to build up complex shapes. I've found that a lot of fun when constructing shapes for raytracing and 3D printing.

dbrgn commented 9 years ago

If you want to take this "solid 3d modelling" approach, you could get some inspiration from programs like OpenSCAD http://www.openscad.org/. I'm pretty convinced that this should be a separate library though, or at least a separate package.

I already commented on this in IRC, but in my opinion there should be a clear distinction between "basic features" that are provided via the API and high level helper functions. It is much easier for a beginner to think about "put blocks at coordinates xyz" than to "create a box instance, intersect it with another box and then do a transformation on it".

I really like the idea of some kind of 3d modelling library, but let's not mix it up with the low level commands :)

ghickman commented 9 years ago

I think first and foremost this library should be focusing on creating an easy to use abstraction for new coders.

With that in mind it would be a good for us to start small and focus on the up front API before adding more complex features. @jonathanfine's suggestion of looking at example code seems like the best idea here and the link is a great start.

One of the things that stands straight away to me is the import structure. I'd like to make everything available from minecraft, eg:

from minecraft import Minecraft

Working with this idea of clean imports I think we can improve the connection interface by hiding any complexity by default, much like how requests does with its Session objects:

mc = Minecraft()

Then sending a message to the server should try to mirror how people play the game:

mc.say('Hello World')

Taking these ideas and running with them, getting the player position and working with blocks should be something like this:

mc.get_position()

from minecraft import Block
block = Block('cobble')

I think we can already implement a lot of this by modifying the existing codebase.

What do people think of this API?

jvlomax commented 9 years ago

Something like this is what we decided on during the sprint. We don't have to implement a lot of new stuff yet, just clean out the smelly java-esque stuff and make it look like a beautiful summer romance of python and minecraft.

I'm going to write a quick API guide on the wiki tomorrow on what we decided, iinclduing what @jonathanfine wrote to help visualize the world and how to manipulate it . There are still many things to be decided, but I think it is important that we can all see the basics first, agree on them, and build from there

On 25. september 2014 23:32:04, George Hickman wrote:

I think first and foremost this library should be focusing on creating an easy to use abstraction for new coders.

With that in mind it would be a good for us to start small and focus on the up front API before adding more complex features. @jonathanfine https://github.com/jonathanfine's suggestion of looking at example code seems like the best idea here and the link is a great start.

One of the things that stands straight away to me is the import structure. I'd like to make everything available from |minecraft|, eg:

from minecraft import Minecraft

Working with this idea of clean imports I think we can improve the connection interface by hiding any complexity by default, much like how requests does with its Session objects:

mc = Minecraft()

Then sending a message to the server should try to mirror how people play the game:

mc.say('Hello World')

Taking these ideas and running with them, getting the player position and working with blocks should be something like this:

|mc.get_position()

from minecraft import Block block = Block('cobble') |

I think we can already implement a lot of this by modifying the existing codebase.

What do people think of this API?

— Reply to this email directly or view it on GitHub https://github.com/py3minepi/py3minepi/issues/5#issuecomment-56887361.

dbrgn commented 9 years ago

I also like the concept of putting everything directly in the Minecraft class, but I'd add another namespacing layer. It just makes the different parts more clear.

In pyminecraft the following API is used (quite similar to the internal protocol):

from minecraft import Minecraft

mc = Minecraft()

world

chat

player

camera

The function names are totally up for discussion, but I like the namespacing because it allows you to use more concise function names. I find mc.camera.set_third_person more clear than mc.set_camera_third_person. To quote the zen of Python: Namespaces are one honking great idea -- let's do more of those!

The implementation is done by using command classes per category. Common functionality like the connection handling can then be abstracted into the common command base class. Because all these command classes are instantiated at the beginning and attached to the Minecraft instance, the user does't see anything of this and doesn't need to import or instantiate any commands or anything like that.

doismellburning commented 9 years ago

Namespaces++

Incidentally I would like to not break the existing API unless absolutely possible. I am happy to maintain a set of wrappers in as-close-to-the-old-style-as-possible, but I want minimal breakage for people's existing code. I will try to put together a test suite for this next week.

pozorvlak commented 9 years ago

+1 to more namespaces.

On Fri, Sep 26, 2014 at 7:36 PM, Kristian Glass notifications@github.com wrote:

Namespaces++

Incidentally I would like to not break the existing API unless absolutely possible. I am happy to maintain a set of wrappers in as-close-to-the-old-style-as-possible, but I want minimal breakage for people's existing code. I will try to put together a test suite for this next week.

— Reply to this email directly or view it on GitHub https://github.com/py3minepi/py3minepi/issues/5#issuecomment-57002732.

ghickman commented 9 years ago

@doismellburning – the plan is to provide full backwards compatibility, which is likely to involve some __init__ tricks but that would mostly limit changes to switching mcpi to minecraft.

doismellburning commented 9 years ago

@ghickman <3 (sorry, still catching up, it's been a busy week)

jonathanfine commented 9 years ago

The existing API describes itself as

Minecraft PI low level api v0.1_1

I think we need a low-level API and a high-level API (supporting things like setting the table for dinner #9). They have different purposes. I've been thinking mostly about the high-level API, but over the weekend I've had some thoughts about the low-level API. In particular, I think we need legacy and replacement low-level APIs - see #10.

dbrgn commented 9 years ago

I think this issue can be replaced by the following issues:

Reopen if you disagree.