Unit tests and OpenVIII

[FlameHorizon]

Is there any reason why this project does not have unit tests?

I was looking at Core/Saves/ATBTimer.cs and thought - I might start my help here by adding unit tests to this class and continue refactoring.

Are there any red flags regarding unit tests in this project which I'm not aware of?

Ps. I'm aware about this Testing suite but I can't get it to work.

[jonasrk]

If I see some examples for unit tests, maybe I can also help with adding a few. :) Having some unexpected freetime. ;)

[Sebanisu]

Someone tried to add unit tests. Though I donno how to use them.

[FlameHorizon]

Today’s update (mostly to clear my head) - I started working on test cases for Core/Saves/Damageable.cs and Core/Battle/Enemies/Enemy.cs which inheritances from Damageable. And oh boy, the dependencies are really. Although I’ve managed to write a few test cases for public bool Damageable.ChangeHP(int dmg). I hope either tomorrow or on Saturday Sunday I will have some to show.

[FlameHorizon]

Today I’ve slowly started making some non-breaking changes in #163 . This will help me to understand project and people who are working on it better.

To the owners - can I help you in any ways implement a coding convention for this project?

[Sebanisu]

I had been using reshaper and codemaid to try to clean up some code the last few weeks. I haven't committed to master yet. I'm not sure if there is a way to define the conventions. Though reshaper has some default ones I was using. I like reshaper because I load a file and everything is highlighted like spelling errors and things that don't follow the naming convention. So then I just need to go through the file and make it happy. So then I get a green checkmark. Codemaid just sorts everything and groups the methods, fields, properties, etc. I'm kinda a noob. I know like c++ has a clang tidy file. I don't know if something like that exists for c#.

[FlameHorizon]

I've moved discussion about coding style to a separate issue #165

[FlameHorizon]

Great effort! I will start writing today.

[FlameHorizon]

Update. Now, with this recent PR, it is even harder...

[Sebanisu]

harder to test?

[FlameHorizon]

Is there a reason why do you have a static CreateInstance methods with private or protected constructors?

A lot of classes (like Saves.CharacterData) are contected with global static class Memory .

Class Saves.CharacterData contains information about character's data as well as operations on binary data. Probably there are just too many things which are happening in one place.

[Sebanisu]

I am told new keywords should be hidden behind a factory method. https://refactoring.guru/design-patterns/factory-method/csharp/example

Though maybe I just did a poor job at implementation.

[Sebanisu]

Only should be two ways to make a character data instance. Loading a save or reading init.out. Enemy data stored in enemy dat files so they're loaded at the start of battle. You could have the test run after loading init.out or just feed the code random binary data and it will break and throw errors. Maybe we could break the monolith class into smaller sub classes. I was wondering if we should get rid of the damageable abstract class. It's job could be handled by an interface. I wanted to have atb and hp functions universal between the entities.