More fixes for backwards-compatibility violations

[doismellburning]

No description provided.

[doismellburning]

For context, plenty of commands still don't work/work well - I'm going to use this PR to test a bunch of command assertions and associated fixes to get the library to hopefully Actually Work With Minecraft...

[doismellburning]

Tests will fail; chat is currently broken

[coveralls]

Coverage increased (+1.04%) when pulling 8ea239d on feature/fix-compatibility into 20790ea on master.

[coveralls]

Coverage increased (+1.04%) when pulling aafa432 on feature/fix-compatibility into 20790ea on master.

[coveralls]

Coverage increased (+1.04%) when pulling aafa432 on feature/fix-compatibility into 20790ea on master.

[coveralls]

Coverage increased (+1.04%) when pulling f7468b1 on feature/fix-compatibility into 20790ea on master.

[doismellburning]

Spectrum Is Green!

[doismellburning]

I still need to test this on Py3 on the Pi, but I think if this works, we have a v1...

[doismellburning]

(I had fun with a lava tower on Python 2 on the Pi!)

[ghickman]

Tests are green and this covers the legacy API which means we've covered our first three goals.

With that I think we're ready for a 1.0 release!

[ghickman]

@doismellburning – Are you happy to roll the release for this?

[doismellburning]

@ghickman Very happy, on Saturday when I'm not in Italy and on a slightly less damp string to the internets

[pozorvlak]

Woohoo!

[jonathanfine]

Sorry to be a damp blanket, but I'm not happy with this being a release.

[doismellburning]

@jonathanfine If you could please not just blindly object without explanation - "I disagree with X" is something people can actually engage with; "I disagree, EOM" is not

[jonathanfine]

I was pressed for time, yesterday. All I had time to do then was give notice of dissent.

For a start, connection.py still has the awful drain method, even though I've submitted code that does the Connection class much better.

In addition, the Minecraft class still takes an address and port as constructor parameters, rather than a connection object. Because of this design decision, it becomes necessary to mock connection when testing, and hence a connection._send method solely for the purpose of

easier mocking and testing.

Because Minecraft hard-codes its dependency on the Connection class support for mocking is necessary in the Connection class. In other words, and you might not agree with this summary of the situation, because Minecraft misuses Connection we add to Connection support for mocking.

That's all I have time for today.

[doismellburning]

I've submitted code that does the Connection class much better.

Could you make that a PR against master please, with just the Connection improvements?

In addition, the Minecraft class still takes an address and port as constructor parameters, rather than a connection object

Yes.

Our main initial goal was to add Python 3 support without breaking compatibility, to stop teachers being in the position of choosing between "teach Python 2.7" or "teach Python 3 but then handle 'yeah sorry this is a bit different'".

We have achieved this goal, and while other goals included improving the API, improving testability, et cetera, the point is that we have met this core initial fundamental goal, ergo a release.

because Minecraft misuses Connection we add to Connection support for mocking

I'm afraid I don't understand what you mean here

[jonathanfine]

Could you make that a PR against master please, with just the Connection improvements?

Please use the already existing PR #49 to discuss the Connection improvements (which can then be seen in context) and, if you wish, express approval.

I'm afraid I don't understand what you mean here.

Please look at https://docs.python.org/3/library/json.html. In particular, note that it does NOT contain load and dump methods that take a filename as parameter.

[jonathanfine]

@doismellburning

Our main initial goal was to add Python 3 support without breaking compatibility, to stop teachers being in the position of choosing between "teach Python 2.7" or "teach Python 3 but then handle 'yeah sorry this is a bit different'".

I'd be happy to support a release that is the same as supplied on the raspi, except that it also works with Python3 - for exactly the reason you state (which was stated by Carrie Anne at PyConUK this year).

We have achieved this goal, and while other goals included improving the API, improving testability, et cetera, the point is that we have met this core initial fundamental goal, ergo a release.

I'm not willing to support a release that contains 'etcetera' just because it also contains something that meets a core goal.

In #10 Low Level APIs I wrote

I think we need two low-level APIs.

The legacy API, which is the current API ported to Python 3.
A properly documented and tested replacement to the legacy API.
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.

@dbrgn and @ghickman expressed clear and strong support for this proposal, and @bennuttall also expressed support.

I'd be surprised if we can't agree very soon on the legacy API ported to Python 3.

I'd be surprised if we could agree quickly on the

A properly documented and tested replacement to the legacy API.

I don't want the release to embody decisions we've not yet made relating to the replacement low-level API. I do want the release to contain the work on the porting to Python 3 (which is the core initial fundamental goal).

[ghickman]

@jonathanfine – To be more clear on @doismellburning's "et cetera" a 1.0 release of the code in its current state will include exactly what you are happy to support:

a release that is the same as supplied on the raspi, except that it also works with Python3

The legacy code has been ported to Python 3 and we now have tests to show it has backwards compatibility.

---

If you have qualms about areas of the code base which you believe are not backwards compatible then please raise them in a new issue. We are not looking to improve legacy code for the 1.0 release.

[dbrgn]

I'm also for a first release that is simply "a mcpi clone with Python 3 support". The next release should then contain the new low level API and maybe improved docs and examples. And then we could start concentrating on the high level API.