Code Formatting

[LucidCrux]

ED, do you have any kind of formatting scheme you use? I'd really like to clean up some of the files if that is okay, but since you're the lead I want to stick to your style. Problem is, I can't nail it down. Sometimes things have opening braces on the same line, sometimes on the next, sometimes things are wrapped and sometimes not, etc. Javascript is kind of a pain for formatting anyways with the multiple ways you can do and declare things.

I'm not especially picky about where braces go or tabs vs spaces for indentation or anything, but I know some people are. If anything, I can just use what seems most common. Mostly, I just noticed it was really hard to navigate the boost file and would like to get more consistency and some whitespace to make things easier to skim and find.

[eternaldensity]

Yeah the boosts are especially badly inconsistent. I'd started off putting boosts all on one line and then they started getting longer and more complicated and having function definitions. Opening braces ought to be on the next line (I think it's clearer) though I like to do } else { as an exception to that because splitting that across lines really wouldn't be necessary. But it's probably best to wait until I put out the next version to ensure it doesn't mess up any current pull requests.

[GeneralYouri]

I fully agree that just like how the code needs some more comments and better variable names, it also needs some better formatting. But at the same time I've never been a fan of putting brackets in their own line, I don't see any reason to. Whenever I open a brace, I also add a tab onto the bracket content, which to me is easily enough to see what's going on there. I do feel that a couple of extra spaces in the lines here and there can help alot (like spacing out parts of an if condition).

[eternaldensity]

Braces on my own has been my habit but yeah it is a kinda silly one so if it got reformatted to not have that, that would be fine.

[LucidCrux]

Cool, I will come back to this later on then.

[eternaldensity]

Okay, later is now :D

Once this is done, I would like to do some refactoring to separate the Sandcastle Builder content from the 'game engine' so I can more easily reuse it.

[LucidCrux]

Alright, getting the last bugs worked out of panel borders, hopefully none are too stuborn, then I'll run through it.

[LucidCrux]

Slighty more in the way of refactoring, but is there a reason some descriptions are functions with simple strings returned involving a Molpify() in them while others are simple the string construction without being inside a function?

Example:

desc: 'Have a Glass Chip production rate from Glass Tools of at least ' + Molpify(20000) + ' Chips/mNP',

vs

desc: function() {
            return 'Spend ' + Molpify(DeMolpify('200M')) + ' Castles total';
        }

[waveney]

THe first case is static, the second dynamic. The second will always work, the first may not be quite right if you switch betwen scientfic and non scientific notation, probably does not matter for small numbers, but for large the second should be used.

[LucidCrux]

That's what I figured, but then there are cases of the first using things like Molpify(1e47). :P Eh, oh well, pretty minor, if someone wants to they can fix em I guess.

[LucidCrux]

Alright, I have put up the first reformated file (badges.js). Please take a look at it (through the network link, no pull request yet) and let me know if there is anything people really dislike about it. It is just a slightly modified version of the eclipse default format to try to match existing code, with a wrap length of 120, plus a bit of hand done changes as needed.

It is perhaps not the best file for seeing the various formating stuff, but there are a couple chunks of code in there for at least a bit of reference outside the simple badge declarations.

[waveney]

I have looked - clearer code yes definately.

Not sure if the change of format from one to many lines for the simple badges is worth it though.

You did not fix the indentations for the quadbadges - which are not consistent.

We haven't added many badges recently (other than discoveries), however I am thinking of a few at the moment. For discoveries beyond the current ones, we need to think how to handle beanish, I have a few ideas, but not attempted to implement any yet.

[LucidCrux]

Odd, the differing tabs on quadbadges didn't show up for me anywhere but on here so I hadn't noticed. Must have been a spaces vs tabs thing. The settings for what I'm doing now is to use actual tabs, not insert spaces for tabs, since that seemed to be what was used most often before.

For the simple badges, yeah, it didn't really make a big difference, but it does slightly improve scanability and will be consistent with other files like boosts where it does help out a lot. I think single lines would have been fine if they were somehow grouped and organized like quads are with np order, but they are really quite random. Mostly though, I just did it for consistency.

[waveney]

The badges are in the order they were thought up (like boosts), this can't change as the load/save depends on this.

[LucidCrux]

Yeah, that was kind of my point. There isn't a way to chunk them together and make nice condensed groups of related declarations. At any rate, badges aren't likely to change much after they're created so a big text wall of badges would probably have been fine, it's mostly just about consistency. :)

The more functional parts are way more important for people to be comfortable scanning and so what I want to make sure you guys are okay with. Like I said, badges.js probably wasn't a great file for this, but it was small enough for me to get through last night. I'll run boosts through the formatter tonight and see how far I get with needed manual changes, that should be a much better reference for what code will look.

[eternaldensity]

All the molpifies should really be in functions rather than just strings, so that when you change the related options they'll change.

There is technically a way to reorder how badges/boosts display (I haven't really tested it though) but yeah unfortunately they can't be reordered in the file without breaking your save. I suppose we could get around that but the only ways I can think of doing that are just as messy in their own way.

[GeneralYouri]

Aslong as order and ID match, you can make some changes in the save parser to simply use IDs instead of order, no?

[eternaldensity]

Aslong as order and ID match, you can make some changes in the save parser to simply use IDs instead of order, no?

Yes. However currently the IDs are all set implicitly by the order so it would be a pain to put them all in manually before it's safe to reorder things. Doable though, I guess.

[LucidCrux]

I think that would be a long and tedious undertaking of it's own. I share ed's enthusiasm for it. Not sure it would ultimately be worth it. I'd guess 90% of the time you do a search anyways to find the name of the boost/badge you're looking for.

And an update: I am about 1/3 of the way through boosts. Unfortunately auto-formating javascript is not as easy/intelligent as c/c++ like languages so there is a good chunk of manual stuff still to fix. Taking a little break then will get back on it, should finish boosts tonight, though.

[eternaldensity]

I'd guess 90% of the time you do a search anyways to find the name of the boost/badge you're looking for.

At a minimum. Actually now I just use NPP's 'find in all open files' all the time.

[LucidCrux]

Unless anyone has changes or anything, I believe this is closable.

FYI type stuff: Line wrap was set to 120, but I tried to keep equations, method calls, and dot.methods, and conditions on the same lines if they weren't absurdly long.

Some of the weirder stuff, like MakeRedundancy() especially, I skipped to avoid messing with existing formating even it didn't match what I was doing.

What else....um... blank lines around functions or declarations with functions (except new badge/boost/tool), and spaces around all operators/comparators.

Can't think of anything else really.

On a completely unrelated note, castle.js and data.js are kind of big messes without clear separation of purpose or grouping. I think I'll let someone else refactor those if they care that much. :P Probably goes along with your engine/content stuff.

[eternaldensity]

On a completely unrelated note, castle.js and data.js are kind of big messes without clear separation of purpose or grouping. I think I'll let someone else refactor those if they care that much. :P Probably goes along with your engine/content stuff.

Yeah that sounds right.