search

mage / mage-sdk-unity

MAGE client SDK for Unity
MIT License
2 stars 13 forks source link

First version #1

Closed MiLk closed 9 years ago

MiLk commented 10 years ago

It should be possible to improve the installation/integration process, but the code should be good.

It requires https://github.com/mage/mage-sdk-cpp/pull/22

nullorvoid commented 10 years ago

I had a look through the PR, a few things that I noticed.

  1. The use of If/else defs is a lot of separation that although doesn't separate functionality it separates meaning, I think that they are overused and could do with some sort of abstraction layer. Nothing overly complex but maybe using factory pattern to generate the correct class for each situation. You could then use the if/else to create the correct concrete class for mobile/android/etc.
  2. Lack of linting, there are a few code style inconsistencies such as missing {} and spacing that I noticed throughout the file. I have not specifically gone and highlighted each one as discussed we do not have a tool to do this, however I think we should look at running something over this code, either in xcode / monodevelop / Visual Studio depending on which development environment we use.

These were the major issues, I couldn't see any other problems with the code itself although I could do with another proper look through it.

The first point also applies to the C++ PR

MiLk commented 9 years ago

This PR can be closed, all the code lives in https://github.com/Wizcorp/ardritactics