Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPG Saga #13

Open
wants to merge 7 commits into
base: Kiselev_Artem_Andreevich
Choose a base branch
from

Conversation

RedMarshall37
Copy link

No description provided.

@jskonst
Copy link
Contributor

jskonst commented Nov 8, 2023

А не убирали файлики из каталога .github? почему-то не запускается проверка

@RedMarshall37
Copy link
Author

А не убирали файлики из каталога .github? почему-то не запускается проверка

Вроде исправил, посмотрите пожалуйста

import { Logger } from './logger';

export class Character {
className = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это зачем,


export class Character {
className = '';
private isBurn = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если завтра появится отравление, заморозка? в принципе любой флаг - это звоночек, что может быть стоит переделать

return this.isBurn;
}

protected isStunned = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично вот тут

Comment on lines +69 to +74
this.health = MathHelper.genrateRandomNumber(this.minHealth, this.maxHealth);
this.strength = MathHelper.genrateRandomNumber(this.minStrength, this.maxStrength);
this.dexterity = MathHelper.genrateRandomNumber(this.minDexterity, this.maxDexterity);
this.characterName = this.selectColorName(names[MathHelper.genrateRandomNumber(0, names.length - 1)]);
this.manaRegeneration = MathHelper.genrateRandomNumber(this.minManaRegeneration, this.maxManaRegeneration);
this.classSkillCost = MathHelper.genrateRandomNumber(this.minClassSkillCost, this.maxClassSkillCost);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вот это лучше бы передавать извне, иначе тесты будет сложно написать

this.health -= damage;
}

private selectColorName(name: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

на это тесты точно надо (может и есть уже)

@@ -0,0 +1,89 @@
export const names: string[] = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вокруг этого можно создать get метод - который бы возвращал элемент по индексу и элемент случайный

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

иначе тесты не получится получить

this.phoneNumber = number;
this.year = year;
this.name = name;
private aYear: number;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если это код примера и он не нужен - удаляйте

@@ -0,0 +1,21 @@
export class MathHelper {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

любой helper - звоночек что он может быть куда то в другое место перенесен

this.numPlayers = numPlayers;
}

generatePlayers() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

как на этот метод написать тесты?

ARCHER = 'ARCHER',
}

export const fireDamage = 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а почему этот тут?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants