Rework codebase

[asyncmeonov]

Codebase requires general re-writing to detangle the acquired spaghetti code.

A non-exhaustive list of refactoring tasks

1. Exchange getters/setters for properties
2. Re-implement Memento pattern
3. Standardise code documentation. Current docs contain signatures of people who implemented particular code snippets, which is made redundant by the existence of git blame

[luvies]

Side note, because the refactor will likely involve the moving of a lot of code (to organise it better), git blame will not track the blame from before properly. Taking this into account, do we still want to remove the blame comments?

[asyncmeonov]

I find neither a practical, nor a deliverable-related usage of these comments. They are also sometimes put in summary tags which I personally find highly off-putting. I'd say into the trash they go.

[asyncmeonov]

Some methods/properties are never referenced (e.g. this one in the minigame, both GetSaves and DeleteFile (I haven't seen any "display list of saved games" functionality, did they leave it for us to implement ?) and the Unit property for colour)

[luvies]

The vector one seems really useless, it might have been for the minigame (since that's the only place it's really dealing with vectors), the others might have a use (although they will need adjusting I think when we bring things up)

[luvies]

We also need to remove the reliance on PlayerPrefs, since it is used for things like controls and game options, not game state data.

[luvies]

Because the current system is a tangled mess, it makes working on it very difficult, even more so with adding the new features that we need. I have come up with a proposal on a restructuring of the code base to move logic around and simplify the entire system:

• Game
  • The game class will mostly be a management class, removing the odd reliance on it for some logic
  • Might be best to implement a singleton pattern to allow access to current players, etc.
• Sector
  • Sector will be converted to only provide checking and action processing methods (what can a unit do here? Move a unit to another sector, etc.)
  • It currently has some actual game logic in it, which is an odd place for it (I think), and causes a weird splintering of logic that is difficult to keep track of
  • An example of this is the various methods that deal with moving into a sector with a unit, there are a lot of small methods that can be easily broken due to checking being done in odd places
  • Click event will still be processed by this class, however it makes more sense to bubble these events up and pass them to a single place that processes all actual unit interaction (i.e. player)
  • Sector will have the capability to have effects applied to them
  • The effects will have to respond to events (likely from the Game class)
• Player
  • My best idea for this was to convert this class into an abstract class and have some base logic (top-level unit handling, etc.)
  • Human and AI player would then be implemented via overriding abstract methods (such as a 'click received' event handler)
  • This means human player can handle click events, while AI ones ignore them
  • An abstract method that allows for some kind of 'on turn start' event handling would allow the AI player to spin up a coroutine that then starts the AI turn processing
  • Events like turn ending can be handled via an event that the Player class exposes, and Game latches onto
  • This allows the Player class to have free-reign on how it processes its turn, allowing for modifier to this (e.g. a card that give the player an extra action, a card that removes an action from another player's next turn)
  • Player will have the capability to have effects applied to them
  • The effects will have to respond to events (likely from within this class or from the Game class)
• Unit
  • Will become mostly a storage class
  • Units will have the capability to have effects applied to them
  • These effects will affect the stats of the unit
  • Effects will need to respond to events (end turn, start turn, death, etc.)
    • These events will need to be triggered by something (likely the Player or Game class), meaning that Unit will need to bind/unbind from the events

[luvies]

We need to nail down what stats and effect we are planning to add (or could add, if we could add an effect, we should implement the ability to) so we know how to implement the effects in general. Things like 'bonus attack/defence' or bonus range (within constraints, e.g. controlled sectors only).

[luvies]

I'm starting work on the rework, meaning all code is subject to change. Part of this pass will include adding in the basic foundations for #17. Since I will be altering how we manage the game, I will be including some data in this comment as a reference.

Player Colours

• Red: CD0000FF
• Purple: B100F0FF
• Yellow: CDCD00FF
• Green: 00CD00FF
• White (Neutral): FFFFFFFF

[luvies]

The core rework is now done. The minigame, effect system, and saving/loading have not been reworked yet, however this is next. The effect system (#17) is the next highest priority, as it is now the basis on how a lot of the internals will work. This includes the landmark bonuses and minigame reward. By doing this, we can simplify and generalise much of the things that happen to players, allowing for a more powerful effect system.

[luvies]

Commit c2af184 has implemented the memento pattern, with some following doing some further tweaks to implement it fully. The doc point in the main comment is done as well, since the complete rework will have removed most, if not all, of those comments.

The final items left to do is the minigame logic (since it needs to be altered to work with the effect reward system), main menu logic (since it also needs to be brought in line with the new code), saving/loading logic (since, now the memento pattern is done, it can be implemented), and fixing the unit tests (since they are still trying to test the old code).

[luvies]

The saving/loading logic (3e95cb0) and main menu code (6d2533b) have been implemented/brought up, meaning that only the minigame and unit tests are left.

[luvies]

The game now runs, albeit with bugs. I haven't checked the main menu or the minigame yet, and all the unit tests are excluded from the builds, but it's progress.

[luvies]

The rework is now mostly done. All that's left is testing it to find any remaining bugs, and to fix all the unit tests. Testing can be done by all of us, but I'll work on the unit tests soon.

[luvies]

The minigame has be reworked to play better. More info in #3.

[luvies]

As of b07c455, the unit tests have been completely rewritten to work with the new codebase. I've also added more fixtures to tests parts of the code that now have more logic in and are worth testing, meaning overall coverage is greater.

The only things I haven't tested are the events in Game and the propagation of them, however this is difficult to test and can be mostly tested via gameplay testing. I have also tested the raising of events lower down (in sector and unit), meaning that it's just the propagation and handling by Game that isn't tested, however this shouldn't be difficult to test via gameplay.

Thanks to this final rewrite, the rework is now complete. All future development will be focused on extending the game to accommodate the new requirements and design.

[luvies]

Oh, saving/loading aren't tested either at the moment, however gameplay testing has ensured correctness up till now.