Обучение #39

Merged
lopar merged 15 commits from maksym into dev 2023-01-23 11:45:10 +00:00
Contributor
No description provided.
DarksLight2 added 5 commits 2023-01-15 22:29:17 +00:00
DarksLight2 requested review from lopar 2023-01-15 22:30:05 +00:00
DarksLight2 added a new dependency 2023-01-15 22:30:30 +00:00
lopar requested changes 2023-01-15 23:57:45 +00:00
lopar left a comment
Owner

Ну как-то так пока что.

Ну как-то так пока что.
@ -45,3 +50,16 @@ spl_autoload_register(function (string $classname) {
require_once $file;
});
Owner

Перенося - переноси. в загрузчике классов незачем выполнять игровую логику. 🤔

Перенося - переноси. в загрузчике классов незачем выполнять игровую логику. 🤔
Author
Contributor

не знаю только куда, есть какие то файлы/классы которые на каждой странице есть?

не знаю только куда, есть какие то файлы/классы которые на каждой странице есть?
Owner

А зачем тебе каждый возможный файл проекта?

А зачем тебе каждый возможный файл проекта?
DarksLight2 marked this conversation as resolved
@ -0,0 +6,4 @@
class SecondStep extends StepFactory
{
Owner

Тест - так и задумано?

Тест - так и задумано?
Author
Contributor

это нужно с Евгением говорить чтобы написал тексты, но в целом да

это нужно с Евгением говорить чтобы написал тексты, но в целом да
DarksLight2 marked this conversation as resolved
@ -91,0 +99,4 @@
return $data->$short_name;
}
private function generateToken($length = 16)
Owner

Вроде как полностью дублирует метод PassGen::new().

Вроде как полностью дублирует метод PassGen::new().
Author
Contributor

не знал о таком, но посмотрю

не знал о таком, но посмотрю
DarksLight2 marked this conversation as resolved
@ -94,3 +119,3 @@
{
if(!$this->database_records) {
$this->database_records = Db::run('SELECT * FROM user_training WHERE user_id = ?', [$this->user_id])
$data = Db::run('SELECT * FROM user_training WHERE user_id = ?', [$this->user_id])
Owner

Если нет особой необходимости, лучше использовать стандартный Db::getRows(), для единообразия.

Если нет особой необходимости, лучше использовать стандартный Db::getRows(), для единообразия.
Author
Contributor

Выбрал этот путь, потому что только одна запись там должна быть.

Выбрал этот путь, потому что только одна запись там должна быть.
Owner

Тогда Db::getRow

Тогда Db::getRow
DarksLight2 marked this conversation as resolved
@ -103,0 +137,4 @@
$this->database_records->data->$short_name = $this->firstRecordData()->$short_name;
Db::run('UPDATE user_training SET data = ? WHERE user_id = ?', [
Owner

Db:sql()

Db:sql()
Author
Contributor

А смысл, если этот метод все равно вызывает run?)

А смысл, если этот метод все равно вызывает run?)
Owner

run изначально приватный медод, который cтал публичным потому что где-то в паре мест понадобился доступ к PDOStatement.

run изначально приватный медод, который cтал публичным потому что где-то в паре мест понадобился доступ к PDOStatement.
DarksLight2 marked this conversation as resolved
@ -4,2 +7,4 @@
* @var object $step
*/
$short_name = 'first_step';
Owner

basename($_SERVER['PHP_SELF'], '.php') не?

`basename($_SERVER['PHP_SELF'], '.php')` не?
DarksLight2 marked this conversation as resolved
@ -0,0 +7,4 @@
* @var object $step
*/
$short_name = 'second_step';
Owner

basename($_SERVER['PHP_SELF'], '.php') не?

`basename($_SERVER['PHP_SELF'], '.php')` не?
Author
Contributor

заменю

заменю
DarksLight2 marked this conversation as resolved
@ -0,0 +1,16 @@
<?php
Owner

Точно классы вызовутся без загрузчика?

Точно классы вызовутся без загрузчика?
Author
Contributor

этот файл инклудится, чтобы не дублировать код вынес в отдельный файл

этот файл инклудится, чтобы не дублировать код вынес в отдельный файл
DarksLight2 marked this conversation as resolved
DarksLight2 added 6 commits 2023-01-18 11:55:55 +00:00
DarksLight2 added 1 commit 2023-01-18 12:01:17 +00:00
DarksLight2 added 1 commit 2023-01-18 12:27:10 +00:00
lopar requested review from lopar 2023-01-18 13:11:02 +00:00
lopar closed this pull request 2023-01-18 13:12:55 +00:00
lopar reopened this pull request 2023-01-18 13:13:16 +00:00
lopar added this to the (deleted) milestone 2023-01-18 13:15:31 +00:00
lopar approved these changes 2023-01-18 14:24:00 +00:00
lopar left a comment
Owner

Почему ChatFirstStep.php, ChatSecondQuest.php, ChatThirdQuest.php, MyUserFirstQuest.php, MyUserFirstStep.php, MyUserFourthQuest.php, MyUserFourthStep.php, MyUserSecondQuest.php, MyUserSecondStep.php, MyUserThirdQuest.php, MyUserThirdStep.php - это набор классов с константами, а не статический массив, например, или не записи в базе? 11 классов, которые вообще ничего не делают вот совсем - выглядит очень нехорошо.

По ходу они созданы, чтобы таки стать тупыми элементами массива. Что-то ты тут сильно перемудрил, товарищ.

Почему ChatFirstStep.php, ChatSecondQuest.php, ChatThirdQuest.php, MyUserFirstQuest.php, MyUserFirstStep.php, MyUserFourthQuest.php, MyUserFourthStep.php, MyUserSecondQuest.php, MyUserSecondStep.php, MyUserThirdQuest.php, MyUserThirdStep.php - это набор классов с константами, а не статический массив, например, или не записи в базе? 11 классов, которые вообще ничего не делают вот совсем - выглядит очень нехорошо. По ходу они созданы, чтобы таки стать тупыми элементами массива. Что-то ты тут сильно перемудрил, товарищ.
@ -0,0 +22,4 @@
return [];
}
public function getShortName(): string
Owner

Это методы, возвращающие.. константу?

Это методы, возвращающие.. константу?
Author
Contributor

Это добавляет гибкости. Захочешь ты для определённого задания сделать логику отличную от других, сможешь спокойно это сделать, плюс я уже убедился что это хорошое решение, когда решил переписать логику трейнинга.

Это добавляет гибкости. Захочешь ты для определённого задания сделать логику отличную от других, сможешь спокойно это сделать, плюс я уже убедился что это хорошое решение, когда решил переписать логику трейнинга.
@ -6,2 +14,4 @@
use DarksLight2\Training\Steps\MyUserThirdStep;
use DarksLight2\Traits\Singleton;
use PassGen;
use PDO;
Owner

Прямая зависимость от PDO? Где? Как?

Прямая зависимость от PDO? Где? Как?
Author
Contributor

ого, я даже не заметил, без понятия как оно туда попало)

ого, я даже не заметил, без понятия как оно туда попало)
DarksLight2 marked this conversation as resolved
@ -22,1 +46,4 @@
} catch (TrainingException $e) {
}
$this->user_id = $user_id;
Owner

Я бы от греха подальше проверил бы хотя бы на > 0, а в идеале на валидное значение. Прилетит некондит, привет "всё сломалось".

Я бы от греха подальше проверил бы хотя бы на `> 0`, а в идеале на валидное значение. Прилетит некондит, привет "всё сломалось".
Author
Contributor

В идеале в айдишнике юзера не должно быть значения <= 0
Просто подумай, как такое вообще должно работать, если туда куда не надо попадают не те значения?) и все же я думаю что эта проверка должна быть уровнем высше, чтобы не засорять код глупыми if($u->info['id'] > 0)

В идеале в айдишнике юзера не должно быть значения `<= 0` Просто подумай, как такое вообще должно работать, если туда куда не надо попадают не те значения?) и все же я думаю что эта проверка должна быть уровнем высше, чтобы не засорять код глупыми `if($u->info['id'] > 0)`
Owner

У нас есть класс User на 12 тысяч строк. Можем сначала полностью переделать его, чтобы id точно, гарантированно, никогда не мог быть невалидным. Рефакторинг старого кода, он такой.
Я говорю по своему опыту, что запросто встречал случаи, когда $u->info вообще NULL. И в данном случае код вернёт FATAL ERROR и полностью остановится. Я же предлагаю исключительно на время, пока всё хреново, сделать вариант, когда метод вернёт false и ругнётся не останавливая все процессы.

У нас есть класс User на 12 тысяч строк. Можем сначала полностью переделать его, чтобы id точно, гарантированно, никогда не мог быть невалидным. Рефакторинг старого кода, он такой. Я говорю по своему опыту, что запросто встречал случаи, когда `$u->info` вообще `NULL`. И в данном случае код вернёт FATAL ERROR и полностью остановится. Я же предлагаю исключительно на время, пока всё хреново, сделать вариант, когда метод вернёт false и ругнётся не останавливая все процессы.
@ -99,2 +151,3 @@
return $this->database_records;
Db::run('UPDATE user_training SET api_token = ? WHERE user_id = ?', [
$token,
Owner

Так ты если запрос сюда перенёс, то и переменную хорони, которую ты создаёшь в памяти и без изменений пишешь в базу. :)

Так ты если запрос сюда перенёс, то и переменную хорони, которую ты создаёшь в памяти и без изменений пишешь в базу. :)
DarksLight2 marked this conversation as resolved
api/index.php Outdated
@ -0,0 +63,4 @@
case 'go_back':
$training->previousStep();
$training->store();
die(json_encode(['status' => 'ok', 'message' => 'Вы указал не верный ответ!']));
Owner

Тут либо "вы указали" либо "ты указал". :)

Тут либо "вы указали" либо "ты указал". :)
Author
Contributor

Ну проебался чутка)

Ну проебался чутка)
DarksLight2 marked this conversation as resolved
main.php Outdated
@ -33,3 +36,4 @@
}
use Core\{Config, Database, Db};
use DarksLight2\Training\TrainingException;
Owner

Нужны ли игроку технические исключения?

Нужны ли **игроку** технические исключения?
Author
Contributor

Солгасен, тупанул

Солгасен, тупанул
DarksLight2 marked this conversation as resolved
main.php Outdated
@ -36,0 +39,4 @@
use DarksLight2\Training\TrainingException;
use DarksLight2\Training\TrainingManager;
$training_manager = TrainingManager::getInstance(User::start()->info['id']);
Owner

На dcc6a1337c/main.php (L59) вызывается инстанс User. Зачем вызывать ещё один на 20 строк выше?

На https://src.lopar.space/new-combats.com/game/src/commit/dcc6a1337c861b9f2dfd957aeebc6e1c6d8cc85b/main.php#L59 вызывается инстанс User. Зачем вызывать ещё один на 20 строк выше?
Author
Contributor

Не обратил внимание)

Не обратил внимание)
DarksLight2 marked this conversation as resolved
@ -146,210 +148,228 @@ if (isset($_GET['loc'])) {
) . '" AND `city` = "' . $u->info['city'] . '" LIMIT 1'
)
);
$tr_pl = mysql_fetch_array(
Owner

Куча замен ниже, это тоже про обучение? Костыли, увечья, хаосники, турниры..

Куча замен ниже, это тоже про обучение? Костыли, увечья, хаосники, турниры..
Author
Contributor

Я не понимаю почему это добавилось, наверное потому что я обернул в условие. Я хз

Я не понимаю почему это добавилось, наверное потому что я обернул в условие. Я хз
DarksLight2 marked this conversation as resolved
@ -441,6 +444,11 @@ $tma = '';
WHERE
`id` = '" . (int)$u->info['id'] . "';"
)) {
TrainingManager::getInstance()
Owner

А если человек поднимет не стат, а навык или особенность? Он пройдёт квест так и не подняв стат.

А если человек поднимет не стат, а навык или особенность? Он пройдёт квест так и не подняв стат.
Author
Contributor

упс, не заметил, я думал там только ability меняется

упс, не заметил, я думал там только ability меняется
DarksLight2 marked this conversation as resolved
@ -3,4 +4,3 @@
{
die();
}
require_once('/home/newcom1/public_html/_incl_data/__config.php');
Owner

Убирая где-то __config.php и __db_connect.php надо вместо них вызывать Config::init() и Database::init(). Первое подтягивает настройки, второе подтягивает функции mysql_.

Убирая где-то `__config.php` и `__db_connect.php` надо вместо них вызывать `Config::init()` и `Database::init()`. Первое подтягивает настройки, второе подтягивает функции `mysql_`.
Author
Contributor

Я же убирал галочку (

Я же убирал галочку (
DarksLight2 marked this conversation as resolved
@ -0,0 +1,42 @@
<?php
ini_set('display_errors', 1);
Owner

Не надо такое прописывать в отдельных файлах, чтобы потом общие настройки игнорировались и перезаписывались.

Не надо такое прописывать в отдельных файлах, чтобы потом общие настройки игнорировались и перезаписывались.
Author
Contributor

Я для себя ставил и забыл убрать(

Я для себя ставил и забыл убрать(
DarksLight2 marked this conversation as resolved
@ -0,0 +16,4 @@
$manager = TrainingManager::getInstance();
$step = $manager->getRegistered()[$short_name];
$button_text = 'Продолжить';
Owner

Если это константа, тогда зачем нужна переременная? Сразу в скрипт её.

Если это константа, тогда зачем нужна переременная? Сразу в скрипт её.
Author
Contributor

Я там думал менять текст при вопросах, квестах и просто информационных блоках, но забыл)

Я там думал менять текст при вопросах, квестах и просто информационных блоках, но забыл)
DarksLight2 marked this conversation as resolved
@ -265,6 +266,18 @@ if (isset($_POST['msg']) && str_replace(' ', '', $_POST['msg']) != '') {
mysql_query("UPDATE `chat` SET `delete` = 1 WHERE `login` = '" . $u->info['login'] . "' LIMIT 1000");
$_POST['msg'] = 'Я спамер ' . $u->info['login'] . ' и меня нужно заблокировать https://new-combats.com/info/' . $u->info['id'] . '';
$training_manager = TrainingManager::getInstance();
Owner

И эта штука один раз отработала и дальше будет вхолостую колбаситься на каждое сообщение в чате?

И эта штука один раз отработала и дальше будет вхолостую колбаситься на каждое сообщение в чате?
Author
Contributor

Я не знал как по другому, может не додумал

Я не знал как по другому, может не додумал
lopar requested review from lopar 2023-01-18 14:24:32 +00:00
DarksLight2 added 1 commit 2023-01-18 17:24:37 +00:00
DarksLight2 added 1 commit 2023-01-18 19:21:20 +00:00
lopar merged commit 6095358496 into dev 2023-01-23 11:45:08 +00:00
lopar deleted branch maksym 2023-01-23 11:46:13 +00:00
Sign in to join this conversation.
No description provided.