TOGoS / TMCMR

TOGoS's Minecraft Map Renderer
http://www.nuke24.net/projects/TMCMR/
Other
45 stars 18 forks source link

First version of Base32 test, with some questions left. #33

Open rkiddy opened 7 years ago

rkiddy commented 7 years ago

I am interested in trying to help extend some of this application and I thought I would start by adding tests. Some people prefer to see larger PRs, with everything to be done, and some prefer smaller, with an interactive back and forth going on. Let me know which you prefer. I can manage this PR for as long as you prefer and for whatever purposes.

My ideas revolve around adding something that can highlight certain blocks. For example, if I can highlight minecart track blocks (and related), I can see a "railroad map" of my world. Also, perhaps something to highlight torches, so that I can see an "explored caves and passages map". Also, I think a few things may be better documented but I am still figuring out what is here.

Please comment with your thoughts. I am also at ray@ganymede.org.

cheers - ray

piegamesde commented 7 years ago

I think the original author of this application lost interest and does not actively maintain it. My pull request #32 which contains majors refactoring and a lot of new features did not get merged yet.

Concerning the test, you write this line (and variations of it) a lot:

assertEquals("foo", new String(Base32.decode("MZXW6")));

I'd suggest you to put this code in a method like test(String normal, String base32, boolean shouldBeSame). That would allow to add more test cases more easily.

TOGoS commented 7 years ago

Hi @rkiddy. To answer your question, I prefer little pull requests. :) As for the highlighting, if I understand what you're going for correctly, you could accomplish that by making a custom block colors file where everything except the blocks of interest are dimmed or made partially transparent (generating that alternate block colors file could be a pain, but that's why Perl was invented).

As for this one, I didn't bother including a Base32 test because I copied the implementation from another project where it was tested, but that doesn't mean I shouldn't have one here if it gives people more confidence in the code. But if I may make a few requests:

In some circumstances padding is not required or used (the padding can be inferred from the length of the string modulo 8). RFC 4648 states that padding must be used unless the specification of the standard referring to the RFC explicitly states otherwise. Excluding padding is useful when using base32 encoded data in URL tokens or file names where the padding character could pose a problem. -- https://en.wikipedia.org/wiki/Base32