command-and-conquer
command-and-conquer copied to clipboard
adityaravishankar
Coding standards
Hi Aditya,
Wow, fantastic job on this game! Really amazing how close it gets to 'the real thing' ;-) . Brings back some good memories... Thanks for that! All we need now is an EVA installer ;-)
I was reading through your blog where you stated you would be interested in turning this more into a 'community project'. I'ld love the idea of that. So, as a start, i was looking through the source and had some ideas.
First of all, i think as a team project there should be some sort of 'coding standard'. Jslint is pretty strict so imho a good starting point. This commit gets the source to comply to this coding standard. (You've made a couple of commits since i checked out and started coding, but i guess it's pretty straightforward for you to re-implement these features, if you like the idea...) A couple of things i fixed along the way:
- Moved 'var' declarations to the top of the functions and remove doubles and added missing ones.
- Fixed indenting, mixed tabs and spaces (that pretty much messed up the 'diff', sorry for that)
- Bugfix in function 'preloadImage' (typ0)
- Added a few todo's , e.g.
- 'Infantery.draw' (function drawSprite doesn't exist?)
- Bugfix in function 'vehicles -> moveTo -> processOrders' (var collDirection doesn't exist?)
- And a few more minor cleanups, no big things...
While scrolling through the code, i had some ideas too i'ld like to share with you:
- What about implementing a nice debugger-console-thingy. This way we can uncomment all alerts() / console.logs(), give them a priority and disable / enable them with a single flag!
- Splitting cnc.js into modules would be nice and give some structure to the code. How about settings up a simple js-loading-mechanism en setup a build process to minify and build releases? Or do you have such a script already? Some node.js script or something?
- And while we're at it, we make it Google Closure compatible to squeeze the last bit of speed outof the browser ;-)
- I did not have enough time yet to look into this, but i have feeling there's pretty much 'code duplication' that could be removed... Are the any quick wins in that, that would make the code more structured too?
- And how about the copyright issues? Any thoughts on that?
Anyway, just interested in your view on this. I love the project and would like to help this to the next step. Can't wait to get the original music back, and how about an online websockets based C&C MMO :-))) Thanks again for your efforts!
Tim
I agree with the general idea of cleaning up the code.... However with 4,290 additions and 4,277 deletions in this pull request, it is impossible to compare and see what is happening...
A lot of points you mentioned (other than the var on top) will go away over my next few planned patches....
For example : The code repetition is happening because I am still working on optimizing certain routines and I need to verify what needs to stay/go away before I rewrite them to use common functions
DrawSprite was recently removed as I found a better way to optimize the sprite drawing. Infantry wasn't changed because the game doesn't draw infantry yet. It will get cleaned up when I add infantry back to the game.
The code is still going to change a LOT. This might not be the best time to try to do this kind of cleanup/rewrite....Maybe after the first major release. A lot of the code you clean up might just disappear in a few days....
If you could break these out into smaller issues and add em to the issue list (there is one for breaking code into modules), it might be easier to implement them in slow steps over the next month while still focusing on adding features....
I agree that this pull request cannot be overseen. But if we want to comply to some sort of coding standard, A LOT of changes in the code would be necessary. The only thing to make it 'oversee-able' if you'ld do it yourself. It's not hard, just run the script a few times through jslint.com . If you have a nice IDE, it would show inline. I don't know what you are using, but as a sidenote: i'm using WebStorm as JavaScript IDE now and it's the best one i've ever seen! (and i'm not related in any way to these guys ;-)). It can be done in 2 hours or so.
On code repetition: how about implementing some object-oriented-like structure? This would be structure to the code, more easy to document and understand for others. Besides, we could introduce the use of some unit testing library (e.g. qunit) so it's easier to work through low level changes...
Even if you change the code a lot, most of the coding standards related changed can be cut-and-pasted or rewritten to it's new form. I don't think this is a reason NOT to do this at this time. And moving the vars to the top of the function and so explicitly describing the scope of each variable is in my opinion very important to keep track of the structure en prevent 'clashing' of 2 variables. Anyway, switching to a new IDE might make it easier to do things along the way...
If i can help in any way, please let me know.
Tim
As a suggestion, perhaps we could incrementally clean it up (hoist vars, etc) instead of all at once. There should be no real performance hit, and this way it is easier for people to vet through the changesets - especially if you make the same type of clean up around multiple files instead of mixing lots of different actions, moving code being one of the hardest to monitor without a good diff viewer (github's does not suffice)
Its a lot of work, but what about thinking of moving the code to ES6 and use es6-module-loader, I'm writing my new RTS game with this.
ping!
I was watching this discussion out of interest. Just wanted to check in. How’s everyone doing?
I've been very busy and have no been working on my gaming projects either, but I am also doing well. :)