nicholas-ochoa / OpenSC2K

OpenSC2K - An Open Source remake of Sim City 2000 by Maxis
GNU General Public License v3.0
4.99k stars 90 forks source link

ES6 Refactor #36

Closed Saeris closed 6 years ago

Saeris commented 6 years ago

Please read the commit logs for additional info on the changes in this branch.

The goal of this PR is to clean up the project to make the code more readable, maintainable, and testable. Most of the code has been re-written for brevity, conformity to modern JavaScript best-practices, reduce repetition and length, and to make use of ES6 syntax to reduce the likelihood of errors due to using outdated or confusing syntax.

I'm opening this mid-way through the process. While most of the work is done, there are still some outstanding bugs that need to be addressed in order for the project to run properly. Right now I believe I've narrowed down the cause of these bugs to the code running in Electron's main process and not in the renderer process, which I aim to address next alongside adding Webpack to bundle the main application. (@TheLarkInn perhaps you'd like to take a look at this PR, as I understand you're interested in adding Webpack to this project as well.)

A few things to take note of:

Please feel free to ask questions! I basically came in and re-wrote the entire project in the last few days, so I don't expect this to be merged without some review and changes.

Saeris commented 6 years ago

TODO:

nicholas-ochoa commented 6 years ago

This is pretty huge and a lot to review - I've skimmed through it and already I've learned quite a few things. I'm working on a refactor of the code to utilize the PIXI.js engine compared to trying to roll my own canvas/webGL renderer. In doing this, I'm definitely going to make use of much of what you've changed or altered as far as code structure and ES6 syntax. I'm also planning to remove the sqlite dependency entirely since it seems to be causing quite a few issues.

I'm using Sublime Text right now, but I should be able to make use of the build tools within it to take advantage of the ES6 linting.

Saeris commented 6 years ago

Check these out then if you're using Sublime:

https://github.com/SublimeLinter/SublimeLinter-eslint https://eslint.org/docs/user-guide/integrations https://prettier.io/docs/en/editors.html

Also I'm not familiar with pixi.js or even how to use canvas for that matter. Just cleaned up mistakes where I saw them. You're right in that you probably don't need sqlite at all. I'd recommend looking into using React to build a game UI in that can just be layered on top of the rendering canvas. There's not much sense in rendering a UI to the canvas imho, given that you're using HTML/CSS/JavaScript anyways.

Might want to look into using Apollo + apollo-link-state for game state management as an alternative to sqlite. Personally I find it a lot easier to reason about than Redux, but I'm also biased towards using GraphQL everywhere possible.

DevNebulae commented 6 years ago

I want to extend on @Saeris's response as to why GraphQL may not be a good idea for this. I absolutely love GraphQL and I love to use it in my personal- and school projects, but when you're going to use it for a game, it becomes a different story. GraphQL is only able to serve textual (read: non-streamable content) and it requires the game to either connect to an external server or have a server present whenever you are playing the game. I think it would introduce a lot of unnecessary overhead and it would overcomplicate the game.

Saeris commented 6 years ago

@DevNebulae Not entirely true. You can use GraphQL with apollo-link-state to manage application state similar to how you would use Redux. I'm suggesting it as an alternative to using SQL to store game data (which in this case is all JSON formatted data anyways). You do not need a server component to use GraphQL.

I will concede that it may not be the best for handling image and audio data, though at the moment all of that is being loaded using the node file system API, with JSON manifests serving as an index for the files. If that were to change to some other more efficient solution for loading game assets, then sure, GraphQL would not be a good way to handle any of that. I'm strictly making a case for using it for game state management (simulation, save data, etc).

nicholas-ochoa commented 6 years ago

I learned quite a lot from the refactor and ultimately used a lot of principles and aspects of it as I've re-written the game from the ground up. I've integrated webpack and eslint, along with moving most of the code to ES6 syntax versus the mixed ES-whatever style I'd been using to throw it all together. The new codebase is written using the Phaser 3 game engine with a WebGL backend versus standard canvas - the performance improvements are night and day better.

I'm closing this PR though as the re-write has diverged significantly from the original code that this was based upon and it didn't make sense to build on top of this refactor (some of the re-write was already in process when you submitted this).

I hugely appreciate the effort and time you put in to this, I can honestly say I never expected anyone to take such an interest in this project let alone contribute to it so significantly. I've learned so much from examining this refactor compared to my original codebase since it was the same functionality - it made the learning process so much faster for me.