Code reformatting (drdev)

[Max mtg]

Please do not change the style of code you that you did not write by yourself.

1. I really don't like to search for significant changes in diffs like this one. http://svn.slightlymagic.net/websvn/dif ... &peg=24214 - There are mixed changes: some are useful (renamed local variables and fixed typos), others are unwanted (those calling a newly added string-assebling method of ZoneType that should not be there)
2. There is absolutelly no reason to change the formatting of if-else clauses. They were perfectly fine written with else on same line as closing bracked for if block. Why do you think this needs to be changed?

[drdev]

I'll move the ZoneType createMessage to a static API function elsewhere in the GUI code. It seemed like an ok place for it at the time, but after adding the "Lang" calls, I agree it isn't.

As for code reformatting in general, having a clean and consistent code style is something that's important to me, so I've tried to clean up code in files I've otherwise touched. Besides the occasional typo, it's almost all whitespace cleanup and adding brackets for single line if statements. Even moving else to its own line is just a whitespace change really, though I realize that still shows up in a diff. The main reason I do it is I find it much easier to read code where the else condition isn't immediately below the final line of the if block above it, plus that way its more consistent with all the places that do put else on its own line.

If you'd like me to avoid code cleanup in non-gui code for the time being while you're working on the refactoring, that's perfectly fine. But I won't promise not to continue to work towards a cleaner, more consistent, and easier to read code base.

[Max mtg]

I am afraid it's too late to move createMessage function anywhere .

No problem about adding/removing spaces around braces, but my code reading routine performs worse when there are the line breaks before 'else', because it looks detached from the if-part in that case.
(that's really a matter of taste and habbit)

Since I am as well highly interested in better code readability (and understand it my way) how would you like my batch-changes of formatting in all the modules I change along the way?

[drdev]

Would it have been so hard to have just moved it instead of completely rewriting all the code I changed? Or running it by me first? You may think you got it working as I had it, but now the case is all off.

Please don't just revert my changes in the future. Talk to me first.

[drdev]

Ok, I just noticed you deleted DeckBox, which is now causing major merge issues with the stuff I'm working on right now. What gives? If you want me to stay out of the code your working with, the least you could do is stop screwing with things I'm working on without even talking to me first.

EDIT: Never mind on this one. I see now that you simply made Deck implement InventoryItem directly instead of needing a separate class, which I had though of doing but didn't want to without running it by you first. I can make this work.

[Max mtg]

Would it have been so hard to have just moved it instead of completely rewriting all the code I changed? Or running it by me first? You may think you got it working as I had it, but now the case is all off.

Are you missing the createMessage method or the splitCompoundWord one in Lang?
The first one should not be there at all. ZoneType should not compose strings or be aware of FServer or even Player.
The second one is about string capitalization, it is not related to sentence or phrase construction, so it could be there in TextUtil... probably.

Drdev, you came up with that minor text case changes, that were oddly placed and added dependencies from game to GUI, the ones that have to be avoided to finally detach game rules from app.
So first things first.
After that there'll be a moment for you to take care of propper capitalization and text composition.

Please don't just revert my changes in the future. Talk to me first.

You are asking too much.

Firstly, it is you who should talk to me about making changes to core and game modules, especially during that separation process.

Secondly, we live and work in different time-zones and find spare hours for Forge at different times. Thus asking someone and waiting for their reply could take up to a day or even more. With that in mind, some minor to middle-scaled changes are implemented first and corrected and/or discussed later (like that DeckBox class you've added).

[drdev]

Fair enough Max. Fair enough. I'll work to fix things up again putting those two functions in the proper locations in static GUI classes. Sorry I got so frusturated at you. I guess I'm just used to it being easier to communicate with others in real-time on projects. I agree that I'd rather do what I have to do now and just work with you later to clean it up, much as you have done vice versa in this instance. I am trying to avoid changes to core stuff as much as possible, but sometimes it's unavoidable. I'll just work on being more patient with you when you push back, which are in your right to do.

[Max mtg]

I am trying to avoid changes to core stuff as much as possible, but sometimes it's unavoidable.

Wish I could believe that!
ColorSet already has the methods you've added to ManaCost in 24229


Will reply later with more details

[drdev]

I am trying to avoid changes to core stuff as much as possible, but sometimes it's unavoidable.

Wish I could believe that!
ColorSet already has the methods you've added to ManaCost in 24229


Will reply later with more details

Why can't those methods exist in ManaCost. What's the harm? It performs better that way than needing to create a ColorSet object to perform the calculation, and that's important for my use case for DeckManagers. Particularly since it's a one line function.