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

Refactor/next method #312

Merged
merged 3 commits into from
Feb 17, 2024
Merged

Refactor/next method #312

merged 3 commits into from
Feb 17, 2024

Conversation

kyvg
Copy link
Collaborator

@kyvg kyvg commented Feb 16, 2024

  • перенёс всю логику с отправкой сообщений в battle log в BaseAction,
  • сделал единый формат для сообщений, убрав копипасту и бесконечные MagicNext, PhysNext...
  • сделал единое форматирование сообщений с уроном

Copy link
Collaborator

@catHD catHD left a comment

Choose a reason for hiding this comment

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

👍 норм порефачил, но меня пугает handleError, который станет прям мусоркой.

if (error instanceof CastError) {
this.params.game.recordOrderResult(this.getFailResult(error.reason));
} else {
console.error(error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

вот сюда будут сваливаться кучей все error, и потом не найдем откуда

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Так наоборот же теперь легко найти по stack trace будет. Раньше все ошибки летели в logger и там просто выводилось сообщение 'Ошибка парсинга строки магии'. И нигде ты их найти не мог, в консоль они не выводились, т.к. типа "обработанные" были.

Сейчас в logger будут лететь только CastError, а остальные выводиться в консоль вместе со всем stack trace. Можно дополнительно 'Ошибка парсинга строки магии' выводить, если хотим сразу видеть, что что-то сломалось

Copy link
Collaborator

Choose a reason for hiding this comment

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

Может какой то маркер придумать? Мб в id admin канала слать или что-то в этом роде?

@catHD catHD merged commit 970812d into master Feb 17, 2024
3 checks passed
@catHD catHD deleted the refactor/next-method branch February 17, 2024 13:31
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