SpockBotMC / SpockBot

High level Python framework for building Minecraft clients and bots.
MIT License
198 stars 47 forks source link

Reorganize spock.utils #59

Closed Gjum closed 9 years ago

Gjum commented 9 years ago

spock.utils is made up by lots of collected code from many independent use cases:

Some stuff is not even used anymore. Definitely needs to be cleaned up, probably by splitting into several spock.utils.* and/or moving classes/methods closer to where they are actually used.

nickelpro commented 9 years ago

"lots of collected code from many independent use cases"

Isn't that the definition of a utility file? It's stuff that doesn't have a home elsewhere that lots of unrelated code uses. BoundBuffer is used everywhere in otherwise completely unrelated modules, it doesn't make sense to tie toghether things like networking, chunk parsing, and data decoders over a single 40 line class. Same for the Info class, which probably shouldn't exist at all, but since it does it makes most sense to have it live in util.

ByteToHex and mapshort2id are debugging functions that have proven valuable enough that I get tired of dropping them into random places when I want to use them so they get to live in util. get_settings is a ghetto implemtation of dict().update and I haven't the slightest idea why I wrote it, I was probably drunk.

Plugin related stuff could maybe be moved to somewhere in spock/plugins because they're inherently tied to spock's plugin loader. When it was just pl_announce, and pl_announce was still a completely optional helper decorator, it didn't make sense to place it in say, the client.py file.

Spock is not a physics framework, vectors and collision boxes are useful though and used by more than just the builtin physics plugin. I don't really see where else they could go

gamingrobot commented 9 years ago

Vector stuff has been moved, the rest of the things are used in more then one place so should still live in util. Also you end up with a cyclic dependency if you move pl_announce into the loader because the loader imports the SettingsPlugin.