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
()
Ответ на: комментарий от 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 ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.