update Wrapper development

[suresttexas00]

Do you have any unpushed development code? I have a working set of 1.8 - 1.9 (snapshot 16w02a) code. I would like to make sure it is on a base of the latest code you are developing.

Also wondering if you are pulling the storage.py updates made by Sasszem?

[benbaptist]

I'm all pushed up, I believe. I'll go ahead and pull Sasszem's PR.

About Wrapper.py as a whole - do you think I should just restart from scratch? Almost everything about it needs a rewrite. I dread working on it because it's just too broken at the moment. proxy.py is finnickey, the UUIDs are just crazy, the dashboard code is terrible, the plugin system could use a few improvements, the config code could be better, etc. Do you think I should just start over from scratch? And if so, should I pick a better name than Wrapper.py when doing so? I want something that doesn't have a .py in the name. :P

[suresttexas00]

That's a pretty aggressive project. Wrapper works. Yes, it needs improvement... However, if you don't have time to fix it... will you have time to REALLY rewrite it from scratch? IF you do; I suggest building each feature one step at a time and don't include code for "future" features until you are ready to fully implement them (lobbies and such). I have noticed you tend to include a lot of "partial" not-quite-fully-implemented features that you don't always get time to finish. I think you have learned a lot about what is working and what is just crap... I personally think it will be easier to fix the existing code base.

You should probably consider where you are heading with copyright issues... Is there some plan you have for wrapper's future that you don't just GPL or something? It seems to me like you have been very "wary" of accepting pull requests and generally "manually" incorporate changes... Some of which were simple straight forward requests like C0ugars "trailing whitespace cleanup"... I was wondering if the "proprietarian" nature of your code was behind this?

[suresttexas00]

OK.. here's something to be aware of too... whether you rewrite or clean up this wrapper: Shadowing __builtins__ like "type" as variable names...

I'm attempting to upgrade the chatbox event a little and allow plugins to modify it. Here is a snippet of the code I am adding:

            if type(payload) == dict:  # return a "chat" protocol formatted dictionary http://wiki.vg/Chat
                chatmsg = json.dumps(payload)
                print("sending Json")
                self.client.send(self.pktCB.chatmessage, "string|byte", (chatmsg, position))
                return False

Unfortunately, this bombs out and tells me that "local variable 'type' referenced before assignment"... Why in the world would it not identify it as the built-in (the same code in Client.parse for rawMessage works fine)? Because here in Server.parse, type is the name of a variable in several places, shadowing the built-in.

This is something I read that I will probably implement into proxy; use trailing underscore after variable to distinguish it and differentiate it from the built-in: I.e. - type_ .

[benbaptist]

Well, to be honest, some of the reasoning to why I don't work on Wrapper.py is much is because I dread it a bit - it's pretty gross code as a whole and it needs a lot of work. A problem that I made is that I let a lot of the code go to hell without doing my regular code cleanups, and it just started getting worse. I don't want to let something like that happen again. If I were to do a re-write, I'd sit down and create some collaborative documents (ones that all of you can work in too) and plan out the code structure, styles, etc. and work out how everything would work BEFORE writing even the first line of code.

To be honest - some of the pull requests, like the trailing whitespace cleanup, are a result of me not using/knowing git properly. I'm only RECENTLY figuring out how to do merge pull requests properly - I previously just copied and pasted crap manually. Something like manual whitespace isn't something I can easily copy and paste, so I got a bit lazy. That was mostly laziness on my part. :P

Ah, yes - there's lots of silly conflicts with built-in methods and variables. It's a very bad habit that I have, and I definitely need to work on - or at least I need to start using builtins more often to be safe. Putting an underscore after a name is ugly, though - just a better name would be preferred.

[suresttexas00]

Hmmm.. I tried prefixing it with __builtins__.type and that did not work. I found the underscore suggestion on stackexchange.

Being a github noob of the 33rd degree myself, I find myself just simply copying the entire file contents and re-pasting it works pretty well sometimes... :D

[suresttexas00]

Any way, I have made good progress with my 1.9 version (which still runs 1.8) and am running my own server on 16w02a. I have also finished the expansion of player.chatbox to be able to modify and return the payload:

        if id == self.pktCB.chatmessage and self.state == 3:
            rawdata = self.read("string:json|byte:position")
            rawstring = rawdata["json"]
            position = rawdata["position"]
            try:
                data = json.loads(rawstring)
            except: return

            ## added code
            payload = self.wrapper.callEvent("player.chatbox", {"player": self.client.getPlayerObject(), "json": data})
            if payload is False:
                return False

            if type(payload) == dict:  # return a "chat" protocol formatted dictionary http://wiki.vg/Chat
                chatmsg = json.dumps(payload)
                self.client.send(self.pktCB.chatmessage, "string|byte", (chatmsg, position))
                return False

            if type(payload) == str:  # return a string-only object
                print("player.Chatbox return payload sent as string")
                self.client.send(self.pktCB.chatmessage, "string|byte", (payload, position))
                return False

            if payload is True:
                return True

Partial plugin sample that adds rank prefixes / suffixes and puts a Google translate link as part of the persons name:

    def playerChatBox(self, payload):
        """ "player.chatbox" event is server chat sent to clients.  This section is mostly concerned with
        intercepting certain notifications to players that are "undesireable", such as telling everyone that
        a vanished op/admin just died!
        """
        player = (payload["player"])
        data = payload["json"]

        # the name of the subject player if this is an administrative chat
        visitorstr = "None"  # the name of another player who is logging in/out, or server sent a death message about
        if "translate" in data:  # special type of chat
            typechat = str(data["translate"])
        else:
            typechat = "None"  # regular chat or broadcast
        if "with" in data:
            if "insertion" in data["with"][0]:
                visitorstr = str(data["with"][0]["insertion"])

        # modify chat some
        if typechat == "chat.type.text":
            # we now know the expected format
            chatmessagetext = str(data["with"][1])
            modifywith = data["with"][0]

            if player.hasPermission("google.translate"):  # give player a google translate link
                translationtext = chatmessagetext.replace(" ", "%20")
                modifywith["hoverEvent"]["action"] = "show_text"
                modifywith["hoverEvent"]["value"] = "Translate this at Google translate..."
                modifywith["clickEvent"]["action"] = "open_url"
                modifywith["clickEvent"]["value"] = "https://translate.google.com/#auto/en/%s" % translationtext

            insertionplayername = modifywith["insertion"]
            chatdisplayname = modifywith["text"]
            #if insertionplayername[0:1] = 0xa7 - gotta get away from ASCII to use

            # insert staff titles and ranks - of course loop would be better- loop through Groups()?
            insertionplayer = self.api.minecraft.getPlayer(insertionplayername)
            if insertionplayer.isOp():
                modifywith["text"] = "%s [OP]" % chatdisplayname
                chatdisplayname = modifywith["text"]  # permanently tag OP suffix
            if insertionplayer.hasGroup("member"):
                modifywith["text"] = "[Member] %s" % chatdisplayname
            if insertionplayer.hasGroup("trusted"):
                modifywith["text"] = "[Trusted] %s" % chatdisplayname
            if insertionplayer.hasGroup("helper"):
                modifywith["text"] = "[Helper] %s" % chatdisplayname
            if insertionplayer.hasGroup("jrmod"):
                modifywith["text"] = "[JrMod] %s" % chatdisplayname
            if insertionplayer.hasGroup("mod"):
                modifywith["text"] = "[Mod] %s" % chatdisplayname
            if insertionplayer.hasGroup("admin"):
                modifywith["text"] = "[Admin] %s" % chatdisplayname
            if insertionplayer.hasGroup("OPS"):
                modifywith["text"] = "[Operator] %s" % chatdisplayname
            if insertionplayer.hasGroup("Owner"):
                modifywith["text"] = "[Owner] %s" % chatdisplayname

            # return any changes
            data["with"][0] = modifywith
            return data

[suresttexas00]

I think maybe you should just tackle each section one by one... Just prioritize fix the worst problems/ easiest fixes first and take them one at a time: 1) UUIDs is a bug of some kind that is generally worsens/manifests with wrapper up-time.. I have basically put a band-aid in this one with daily wrapper halt and restarts. 2) proxy.py (I've gotten a lot done here, if you are interested... I have to diff and merge my changes with build 109) 3) Plugin system (I still have a PR on this...) 4) Log writer 5) config code ... 777) or whatever last is.. fix dashboard. It's a nice thing, but its power windows when our engine still stalls.

[suresttexas00]

I was just starting to get a handle on using GIT / github from windows... I'm back to [] 1 again and need to relearn for Linux.. :P

[JasonBristol]

I don't think a full rewrite is necessary since the wrapper does work, it just feels more like an alpha. We should definitely go through the codebase and perform some cleanup as well as identify areas that may actually need to be rewritten (proxy.py really sticks out). I guess the good news is you wouldn't have to do it alone ;P

Just to add to the priority list @suresttexas00 posted, I really don't see the dashboard as having any kind of priority. The UUID bug is critical and I think that hinges a lot on a proxy rewrite. The plugin system works, but could definitely be extended to do more. I don't really see this as a major priority, but would come before the dashboard. Documentation is almost non-existent, I think it would be nice to flush out at least a basic set of documents. Once we have a more stable build of the wrapper we can begin any kind of refactoring that is desired up to and including the potential to change the name if you really don't like wrapper.py.

@suresttexas00 It sounds like you have done some significant work on the wrapper itself which might be beneficial to look at if you want to make it public. I think the easiest way to do this would be to make a new branch that you could push to and we could start making pull requests into development.

[suresttexas00]

He does have basic documents, but they are quite dated and need updates. API: http://wrapper.benbaptist.com/docs/api.html

Events list: https://docs.google.com/spreadsheet/ccc?key=0AoWx24EFSt80dDRiSGVxcW1xQkVLb2dWTUN4WE5aNmc&usp=sharing

[JasonBristol]

Those docs are more of a technical spec for the API functions, we could really use documentation on the wrapper itself, configuration, examples, use cases, do's and don'ts, etc. Right now we just have a quick API reference for basic plugin development.

When you first check out the wrapper you can get it running pretty quickly as the setup is straightforward, but any serious plugin development and you need to start looking through the code to really get a sense of what is happening. I think this gives it a bit of a learning curve for newcomers. On Jan 18, 2016 10:48 AM, "Surest Texas" notifications@github.com wrote:

He does have basic documents, but they are quite dated and need updates. API: http://wrapper.benbaptist.com/docs/api.html

Events list:

https://docs.google.com/spreadsheet/ccc?key=0AoWx24EFSt80dDRiSGVxcW1xQkVLb2dWTUN4WE5aNmc&usp=sharing

— Reply to this email directly or view it on GitHub https://github.com/benbaptist/minecraft-wrapper/issues/265#issuecomment-172566903 .

[benbaptist]

100% agreed. Would it be ideal to use the Wiki tab on Github itself to write documentation? I can begin to write up some skeletons/stubs.

[JasonBristol]

I think the wiki will work well, will also give us the ability to collaborate On Jan 20, 2016 11:27 AM, "Ben Baptist" notifications@github.com wrote:

100% agreed. Would it be ideal to use the Wiki tab on Github itself to write documentation? I can begin to write up some skeletons/stubs.

— Reply to this email directly or view it on GitHub https://github.com/benbaptist/minecraft-wrapper/issues/265#issuecomment-173261242 .

[suresttexas00]

If you do decide at some point to do a re-write from scratch, I think It would be a good idea to do so in Python 3.

[benbaptist]

I agree. At this point, Python3 is very widely supported, and there's not a whole lot of reason to develop mainly against 2.x. Might still be a good idea to have cross-compatibility, but not a whole lot is needed.

[suresttexas00]

Most Python users are also Linux users too.. and know how to install new packages!

This is Ben.
Ben uses Linux.
Ben has to know a least a little about computers to use Linux,
so Ben knows how to install Python 3.
Ben is smart.
Be like Ben.

[benbaptist]

LOL Yay!

[suresttexas00]

275 done

[suresttexas00]

I think we have licked UUID problems (hopefully).. I do know it will be tons better now. Jason Bristol will be pursuing threading options to close out any possible memory leaks in the threads. I think we are making good progress...

Maybe you could start worrying about your pet peeves like the dashboard code and such.... I'm closing this issue now because the last PR is done.

However, you really want to review and approve the PR #292 ... A much nicer system and I am laying groundwork for more improvements.