search

UFRN-URNAI / urnai-tools

A modular Deep Reinforcement Learning library that supports multiple environments, made with Python 3.6.
Apache License 2.0
5 stars 8 forks source link

Resolves #73 #82

Closed RickFqt closed 3 months ago

RickFqt commented 7 months ago

Resolves #73. When complete, this pull request should have:

RickFqt commented 7 months ago

@alvarofpp For the implementation of TerranState class, I used Simple64GridState class from the original sc2 state classes from urnai as a base. It basically stores information from the player (number of units, barracks and minerals, for example), and also builds a 4x4 grid that stores the location of enemy units. Do you think there is a better choice? I considered choosing UnitStackingState as a base since it is pretty well commented, but it seemed to me that it is too specific for a general purpose class.

Also, I implemented the methods get_my_units_amount and get_units_by_type at the end of the file. Is that right?

alvarofpp commented 6 months ago

@RickFqt Please mark as solved the modifications that you have already solved.

RickFqt commented 4 months ago

Many parts of the code have "magic numbers" (values without an explicit definition). I listed a few in the review, but I think you should create a file with constants to map them all.

Should this new file be on the path sc2/states/ as well?

alvarofpp commented 4 months ago

Should this new file be on the path sc2/states/ as well?

I don't think so. Constants are generally used in many parts of the project, so it is recommended to apply them to a single file or use Enum to give them more context.

RickFqt commented 3 months ago

I don't think so. Constants are generally used in many parts of the project, so it is recommended to apply them to a single file or use Enum to give them more context.

In that case, is the path urnai/constants.py a good choice? On the previous version, it was located on urnai/utils/constants.py, but I think we are not using utils anymore, right?

alvarofpp commented 3 months ago

In that case, is the path urnai/constants.py a good choice?

Yes.