search

ToroCraft / SignEdit

A simple Minecraft mod that allows players to edit signs without needed to replace the sign block.
GNU General Public License v3.0
1 stars 1 forks source link

Configurable sign-editor #1

Closed HoldYourWaffle closed 6 years ago

HoldYourWaffle commented 6 years ago

Since 50f53ce you need to have a sign in your (off)hand to edit a sign using rightclick. I made the required item configurable, even allowing for not requiring an item at all.

frodare commented 6 years ago

Thanks for your interest! However, in this PR please only change or add code relevant to the new feature.

HoldYourWaffle commented 6 years ago

Thought you might want that so I already changed those things in seperate commits. I've made a "cleanup" commit reverting everything not-function related.

frodare commented 6 years ago

That's better, but the formatting is still changed. When making a feature change the code style should be kept the same. Most notably in this PR, indention is 2 spaces, not tabs. Ideally the only changes the diff should show are the code improvements you added.

HoldYourWaffle commented 6 years ago

Whoops didn't even notice that. Must've been my auto-formatter. I've fixed it now 😄

HoldYourWaffle commented 6 years ago

I fixed the minor thingies, although I must say I think some things are a bit "unnecessary". (Double if block with same code in it, multi-line if block with only a return statement after it.)

You were completely right about the private fields though that was just an oversight from me 😄

frodare commented 6 years ago

Yeah, they were "unnecessary", I just prefer to keep format / cleanup changes separate from feature and logic changes. I am not saying one way is better than than the other. :)

HoldYourWaffle commented 6 years ago

Well I guess that makes sense 🤔

frodare commented 6 years ago

Cool, thanks! I will try to get around to building and uploading this soon, I haven't been working on these mods lately.