LINUX.ORG.RU

покритикуйте код; slithercc

 


0

2

Здравствуйте,

сочинил клиента web игры slither.io.

slither.io это мультиплеерная змейка в кот. нельзя врезаться в других игроков.

https://github.com/ivan-matveev/slithercc

Покритикуйте код.

★★

Последнее исправление: imatveev13 (всего исправлений: 2)
for (size_t idx = 0; idx < 24; idx++)
	{
		int32_t value1 = secret[17 + idx * 2];
		if (value1 <= 96) {
			value1 += 32;
		}
		value1 = (value1 - 98 - (idx * 34));
		value1 = value1 % 26;
		if (value1 < 0) {
			value1 += 26;
		}

		int32_t value2 = secret[18 + idx * 2];
		if (value2 <= 96) {
			value2 += 32;
		}
		value2 = (value2 - 115 - (idx * 34));
		value2 = value2 % 26;
		if (value2 < 0) {
			value2 += 26;
		}

Везде магические константы.

seiken ★★★★★
()
Ответ на: комментарий от seiken

Тебе жалко что-ли или завидуешь?

anonymous
()
Ответ на: комментарий от seiken

Да, это в функции decode_secret() кот. генерирует ответ на «секретный» запрос slither.io при открытии websocket. У этих магических чисел нет смысла, я не знаю как их назвать.

imatveev13 ★★
() автор топика
float ang;
uint8_t btn = 254;

ang

btn

Экономия на буковках? Ведь не используется компилятор времен прадедушки с его ограничениями, и мониторы сейчас у людей шире, чем в 80х. И даже если какие-то системщики/старперы/железячники так до сих пор пишут, это не признак хорошего тона.

seiken ★★★★★
()
Ответ на: комментарий от imatveev13

Вообще-то...

Уважаемый seiken прав. Плохая это практика с int_32 и прочим аналогичным. MISRA C тоже со мною согласен (rule 6.3, если не ошибаюсь).

Впрочем, если Вы точно уверены, то почему бы и нет?

Moisha_Liberman ★★
()

где свободная лицензия GNU GPLv3 без плюса,

где самая лучшая система сборки - автотулзы

Harald ★★★★★
()
Ответ на: Вообще-то... от Moisha_Liberman

Думаю seiken ругается на ang вместо angle и btn вместо button.

MISRA говорит использовать типы с размерами.

Rule 6.3 (advisory): Typedefs that indicate size and signedness should be used in place of the basic types.

imatveev13 ★★
() автор топика
Ответ на: комментарий от imatveev13

Значит, да.

Rule 6.3 (advisory): Typedefs that indicate size and signedness should be used in place of the basic types.

Значит, это я туплю. Хотя, сам применяю такие типы, если только точно уверен в том, что мне они понадобятся.

Думаю seiken ругается на ang вместо angle и btn вместо button.

Возможно. И даже скорее всего.

Moisha_Liberman ★★
()
Ответ на: комментарий от Pipidastre

Ну, например, у меня код, который не влезает в 80 символов на строчку, ревью не пройдёт.

В Linux kernel coding style так:

Statements longer than 80 columns will be broken into sensible chunks

так что длина строки только до определенной степени ограничивает длину идентификаторов, и код ТС вроде как не тот случай

seiken ★★★★★
()
Ответ на: комментарий от imatveev13

генерирует ответ на «секретный» запрос slither.io при открытии websocket. У этих магических чисел нет смысла, я не знаю как их назвать.

Если это какая-то кодирующая/декодирующая функция, обычно у нее есть осмысленные параметры и промежуточное состояние (если много итераций одной и той же процедуры), пусть даже если эти параметры названы типа a, b, c, x, y, z. С магическими константами две основные проблемы:

  1. Не понятно, почему именно такие значения работают. Может показаться, что все подобрал как надо, и скажем, 5 минут нормально все работает, а на первой секунде 6 минуты шарики взяли и прыгнули за пределы экрана/улетели в космос; Если же есть формальная модель, ее можно проверить на бумажке аналитически, со всеми граничными условиями и проч.;

  2. Не понятно, если в одном месте одна магическая константа изменена, в каких еще 10 местах что и как надо подправить, чтобы код не сломался. Т.е. это еще и код «только для чтения» и сам автор может забыть через пару лет, какие значения и как подбирались.

seiken ★★★★★
()
Ответ на: комментарий от seiken

После открытия websocket’а сервер slither.io присылает «секретный» пакет кот. надо decode_secret() и послать обратно. Если ответ неверный сервер закроет сокет. Т.е. ситуация: «5 мин. работает на 6ой сдохло» невозможна.

Сам код выдран из оригинального JS клиента. Что автор имел в виду под константами мне не догадаться. Если перестанет работать значит автор изменил «секрет» и надо драть снова.

imatveev13 ★★
() автор топика
Ответ на: комментарий от moonmadness

Если не трудно, приведите корявый кусок и поясните что с ним не так. На мой замыленный глаз приведенный по ссылке код выглядит приблизительно как весь остальной. Хочу улучшить код , но по имеющимся замечаниям не могу придумать как.

imatveev13 ★★
() автор топика
Ответ на: комментарий от imatveev13

Ну, я конкретно про этот if говорю:

	if (
		str_ends_with(target, ".png") ||
		str_ends_with(target, ".jpg")
		)
		return "image/webp,*/*";

А в другом месте так:

	if (snake.head == xy_t{0, 0} && !snake.part_list.empty())
moonmadness
()
Ответ на: комментарий от Harald

судя по всему это «змейка» инкарнированная в web-massive


если разовьётся от «клиент» до «бот», то может быть интересно. С точки зрения алгоритма. Но есть парадокс - в таких простых играх выигрывают простые принципы.

MKuznetsov ★★★★★
()

Посмотрел util.cpp и websocket.cpp. Критиковать нечего - «работает, не трогай».

anonymous
()
Ответ на: комментарий от seiken

Да с чего бы это? Длиные имена даются только глобалкам. Которых не должно быть много. И локалок тоже. Если их много настолько, что нужно много букв, то это и есть говнокод. Сечёшь?

anonymous
()
Ответ на: комментарий от Pipidastre

Ну, например, у меня код, который не влезает в 80 символов на строчку, ревью не пройдёт.

Потому что дело не в ширине экрана и прочем.

anonymous
()
Ответ на: комментарий от imatveev13

О, лицензию забыл, спасибо.

Автотулзы, в маленьких проектах, считаю лишними.

Он приколося. Если что. Кстати, что за лицензия там?

anonymous
()
Ответ на: комментарий от imatveev13

а потом какой-нибудь хитрожоп зарелизит твою программу в проприетарном виде и будет бабло зарабатывать, а ты локти кусать :) Дедушка Столлман плохого не посоветует, GNU GPL v3 (без плюса) - самый разумный выбор

Harald ★★★★★
()
Ответ на: комментарий от Harald

а потом какой-нибудь хитрожоп зарелизит твою программу в проприетарном виде и будет бабло зарабатывать, а ты локти кусать :)

Этот комментарий кажется безхозным. Помещу-ка я его в свой сборник кортких драматических рассказов и буду бабло рубит, а ты локти кусать.

Даже эпилог придумал:

«И умер он от чахотки, в нищите и забвении. А на могиле, отдавая долг стремлениям его жизни, насрала антилопа».

anonymous
()
Ответ на: комментарий от Harald

а потом какой-нибудь хитрожоп зарелизит твою программу в проприетарном виде

Пусть берет, там много дорабатывать до товарного вида.

imatveev13 ★★
() автор топика
Ответ на: комментарий от Harald

плохого не посоветует, GNU GPL v3 (без плюса) - самый разумный выбор

Кстати, если развивать тему упущенной выгоды, о которой и не думал. То BSL подойдёт лучше. Т.к. змея мультиплеерная. И предоставляя её как сервис – просто накаитив на голый сервак – неплохо бы отлистать процентик.

anonymous
()
Ответ на: комментарий от anonymous

змея мультиплеерная. И предоставляя её как сервис

Сервер не мой, сервер slither.io. Мой только клиент.

imatveev13 ★★
() автор топика
Ответ на: комментарий от seiken

и мониторы сейчас у людей шире, чем в 80х

да как же вы достали. шире, два терминала по 80.

t184256 ★★★★★
()

Называется так, будто компилирует C. Написан на языке-технодемке восьмидесятых.

t184256 ★★★★★
()
Ответ на: комментарий от Harald

для такого случая есть AGPL

Она не для такого случая.

anonymous
()
Ответ на: комментарий от anonymous

Да, есть маленько. Согласен. =)))

Хорошо намедни «отметили». Аж забыл старое правило – выпил, не лезь в форум.

На деле, я имел в виду несколько иное. Всё зависит от той платформы, на которой мы используем этот код. Весьма возможно что использование явного указания типа может ухудшить производительность. Собственно, как и говорил выше, и как говорится в https://linux.die.net/man/3/int32,

Use int32 only when the application requires exact 32-bit arithmetic.

Да, типы данных int32_t, int64_t и т.д. и т.п., лучше использовать только в том случае, если они действительно нужны Вам.

Moisha_Liberman ★★
()
Ответ на: Да, есть маленько. Согласен. =))) от Moisha_Liberman

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

Как я понимаю рекомендация:

MISRA Rule 6.3 (advisory): Typedefs that indicate size and signedness should be used in place of the basic types.

повышает надежность кода за счет явного указания диапозона чисел кот. может содержать переменная и уменьшения количества неявных преобразований между переменными. Но может снижать производительность.

Да, типы данных int32_t, int64_t и т.д. и т.п., лучше использовать только в том случае, если они действительно нужны Вам.

В коде безразмерных int нет совсем. Большая часть размерных int это работа с протоколом, там они необходимы. Есть размерные int в decode_secret(). Там есть бинарные сдвиги и хитрые % кот. без указания размера переменных работать не будут. Ну и немножко размерных int по совету MISRA для надежности. Как показал профайлинг самые дорогие вызовы функций SDL2 для отрисовки. Т.е. быстродействие от моих размерных int страдает не сильно.

imatveev13 ★★
() автор топика

Не слушай клоунов который говорят про актуальное ограничение в 80 символов на строку. И то что старпёры из linux kernel так делают тоже ничего не значит.

Оглядываться на других можно, но делать - так как считаешь удобнее.

reprimand ★★★★★
()
Ответ на: комментарий от anonymous

Так и назови enum{MAGIC_A=54,MAGIC_B=86} или/и вместеMAGIC_NUMBERS={MAGIC_A=56,MAGIC_B=89}

Эти магические числа живут в функции decode_secret() выдраной из оригинального JS клиента. Задумка сохранения магических чисел из оригинального JS в том чтобы когда оригинальный JS изменится было легче понять что менять в C++ коде.

imatveev13 ★★
() автор топика

Во-первых, где карта комментарии, Билли? Во-вторых, switch-case и reinterpret_cast кругом – ты упоролся? Зачем тогда ты брал кресты, а не Си?

no-such-file ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.