Итоговый PR: mouseartiom #6
Conversation
…лен using ns std. В GetArtifact() возвращается указатель на артефакт. Добавлены Get/Set.
… into mouseartiom
…бавлен в конструктор Item для генерации из вне.
… into mouseartiom
… into mouseartiom
This reverts commit 1f5d458, where event, item and location were deleted.
database/items_data.cpp
Outdated
| // using unique_ptr for two-dim array isn't a good idea though | ||
| // std::unique_ptr<Event[][]> eventMatrix (nullptr); | ||
|
|
||
| eventMatrix = new std::vector<Event>** [gameData.mapHeight]; |
There was a problem hiding this comment.
почему используется двойной указатель на вектор?
это же очень не очень
database/items_data.cpp
Outdated
| else return NULL; | ||
| } | ||
|
|
||
| void EventsData::PulverizeEvents(std::unordered_map<str, Event>& list) { |
There was a problem hiding this comment.
интересное название метода
database/items_data.cpp
Outdated
|
|
||
| bool ArtifactsData::Init() { | ||
| ArtifactFactory aFactory; | ||
| str path = systemData.resourcesDirectory + systemData.nextLocationName + "/artifacts"; |
There was a problem hiding this comment.
этот путь должен передаваться в конструкторе
/ - это системный разделитель - используйте системную функцию/std::filesystem
database/items_data.cpp
Outdated
|
|
||
| SystemData::SystemData(str _nextLocationName) | ||
| : | ||
| resourcesDirectory("resources/"), |
There was a problem hiding this comment.
это должно задаваться ещё выше
database/session_data.cpp
Outdated
| str writer; | ||
|
|
||
| // hero initialization | ||
| std::string name = "Moleque"; |
There was a problem hiding this comment.
это не должно инициализироваться тут
surfaces/surface.cpp
Outdated
| MapScanner scanner; | ||
| scanner.FillMatrix( | ||
| systemData.resourcesDirectory + | ||
| systemData.nextLocationName + "/" + |
surfaces/surface.cpp
Outdated
| return surfaceMatrix; | ||
| } | ||
|
|
||
| // DO NOT use this! It's full of bugs |
surfaces/surface.h
Outdated
| int mapWidth; | ||
| int mapHeight; | ||
| Matrix<char> surfaceMatrix; | ||
| // std::unordered_map<str, Surface> currentSurfaceList; // currently useless |
| public: | ||
| AccessMap(Coord, Coord); | ||
| ~AccessMap(); | ||
| char** getMatrix() {return matrix;}; |
There was a problem hiding this comment.
используйте умные указатели
например, тут можно использовать array>, так как размер жёстко задан
| int width; | ||
| }; | ||
|
|
||
| /* template<class T> |
eadium
changed the title
eadium
changed the title
| : nextLocationName(_nextLocationName) { | ||
| cout << "systemData initialized" << endl; | ||
| resourcesPath = resourcesDirectory; | ||
| artifactsPath = resourcesPath / nextLocationName / "artifacts"; |
| public: | ||
| AccessMap(Coord, Coord); | ||
| ~AccessMap(); | ||
| char** getMatrix() {return matrix;}; |
|
|
||
| class GameData { | ||
| // stores finished events and other stuff | ||
| public: |
There was a problem hiding this comment.
если это public class, то лучше использовать struct...
| if (!surfaceData.CoordIsValid(point)) return NULL; | ||
| if (!eventMatrix[point]) return NULL; | ||
| if (!eventMatrix[point]->empty()) | ||
| // return eventMatrix[point]->front(); |
| add event to map | ||
| */ | ||
| for (auto i : list) { | ||
| Event &event = i.second; |
There was a problem hiding this comment.
почему reference. Вместо auto i. Можно использовать const auto& i, чтобы не копировать
| Event &event = i.second; | ||
| int counter = 0; | ||
|
|
||
| Coord eventCenter = event.GetCoord(); |
There was a problem hiding this comment.
можно использовать auto, чтобы не писать лишнего. Тип сам выведется
| current.y++) { | ||
| counter++; | ||
| if (!eventMatrix[current]) { | ||
| eventMatrix[current] = new std::vector<str>; |
There was a problem hiding this comment.
Почему, raw указатель на контейнер. Это же фиаско. Используй умные указатели
| } | ||
| } | ||
| } | ||
| cout << "event " << event.GetName() << " was sprayed " << counter << " times" << endl; |
There was a problem hiding this comment.
Лучше использовать логгер
| return surfaceMatrix[point]; | ||
| } | ||
|
|
||
| short SurfaceData::getSurfSpeed(Coord point) { |
There was a problem hiding this comment.
const метод. И вобще везде getter-ы. Const методы
| } | ||
|
|
||
| short SurfaceData::getSurfSpeed(Coord point) { | ||
| return surfaceList.at(getSurface(point)).getSpeed(); |
There was a problem hiding this comment.
Если at не находит элемент, то он бросает exception out_of_range. Нужен try catch
|
|
||
| /* | ||
| // struct for terrain modifiers, contains name and power factor | ||
| struct surfaceModifier { |
There was a problem hiding this comment.
опять комменты в релиз коде
| class SurfaceData { | ||
| public: | ||
| SurfaceData(); | ||
| // ~SurfaceData(); |
| public: | ||
| AccessMap(Coord, Coord); | ||
| ~AccessMap(); | ||
| char** getMatrix() {return matrix;}; |
С момента последнего ревью была проделана следующая работа: