Курилка программистов

[Dennis_Chikin 3 658]

Судя по регулярно происходящему в других темах - таки нужна.
Вот здесь как раз можете спросить про "зачем создали эту тему ?"

 

А также про ООП, про как заниматься моддингом и его устарелости неустарелости,  кто как его себе представляет и т.д.  В общем, для много слов "обо всем". Что в более специализированные темы не лезет, но поговорить давно хотелось и хочется.

 

Но таки да, пп 2.0, 2.1 и даже 2.5 правил здесь вполне таки действуют.

Изменено 30 Января 2015 пользователем Dennis_Chikin

[xStream 86]

 

 

self explained код - это миф. Что можно сделать, чтобы код говорил сам за себя, кроме того, что красиво его отформатировать и дать переменными и функциям внятные имена? Я это в обязательном порядке делаю, но этого совершенно недостаточно.

А у нас на роёне это не миф. Да и вообще, я же там мелким ан нет, не мелким шрифтом написала: до тех пор, пока этого достаточно. В одном месте с меня требовали, например, описывать вплоть до блоков кода. А в другом всем было пофиг. И те и другие благополучно сопровождают проекты до сих пор.

 

 

Например

Ну вот собственно это нормальный пример, который на мой взгляд документирован слегка избыточно (ибо названия функций, но не полностью избыточно - из-за разношерстного интерфейса), но в целом все то, о чем и говорим.

@Malandrinus, про хуки и хаки - само собой, это не относится к тривиальным вещам в принципе.

@abramcumner, да, точно!

 

 

И код будет читаться намного быстрее, т.к. уже знаешь чего от него ожидать.

Для меня быстро читается distance_between, а если ты не знаешь, что это и кто такие сталкеры, то тут не комментарии нужны, а ртфм

[Malandrinus 611]

усложняет его модификацию. Увеличил радиус поиска бандитов - надо править в двух местах: в коде и в комментарии

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

 

Хорошо было бы конечно забацать итератор по онлайновым объектам... Тогда было бы вообще, как в комментариях:

 

аггрегаторы, итераторы

 

Я даже как-то что-то такое начал. Замутил универсальный алгоритм перебора с подачей на вход функтора, задающего что именно ищем: онлайновые объекты, серверные объекты, кастомные. Можно было в цепочку выстраивать и последовательно фильтровать, а на выходе - массив результатов поиска. Потом понял, что увеличиваю количество зла, ибо переборы объектов - зло =) Стёр, отформатировал винт.

 

ЗЫ: шучу, где-то валяется, но уже конечно не найти.

ЗЗЫ: но переборы - действительно зло

[xStream 86]

@Desertir, а мне, кстати, нравится (сам подход с декораторами, аггрегаторами и прочей приблудой). Пришерстить отступами и будет вообще шоколадно.

Изменено 2 Февраля 2015 пользователем xStream

[Desertir 202]

но переборы - действительно зло

Ну в дотнете они есть, все таки там больше за надежность нежели за скорость, да и потом всякие запросы к БД, потом это обрабатывать, иногда ЛИНКу тащит и код выглядит симпатичнее.

call chaining

Не ну эт мы в курсе Просто суть ушла в сторону того, как код выглядит. Сначала я хотел написать не методы, а функции (table.where/table.foreach), но потом передумал, ибо выглядит для меня не очень. Изменено 2 Февраля 2015 пользователем Desertir

[xStream 86]

@Desertir, это называется call chaining. Это есть много где, если постараться и построить нормальный интерфейс.

[Malandrinus 611]

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

 

 

 

тут не комментарии нужны, а ртфм

Вот и вся суть. Написать комментарий - секундное дело для человека, который этот код написал. Искать документацию - на порядки большее время для многих людей, которые будут читать этот код. Впрочем, как я уже говорил, комменты я пишу для себя. Я из себя супер мозг не строю. Через неделю я с гарантией забуду детали того, что ваяю сейчас. А с комментами вспомню сразу. И не устану, не отвлекусь. Я себе же время сэкономлю, написав эти несколько фраз.

[Andrey07071977 18]

Через неделю я с гарантией забуду детали того, что ваяю сейчас

 

Потопил! Такая же история и причина для усердного коментирования. Кстати, давно уже пользуюсь вот этим генератором документации для луа. Очень удобно и быстро составлять документацию в html формате, который к тому же можно подстраивать как душе угодно. Даже если просто запустить утилиту со скриптами сталкера, то как минимум получите удобный список функций.

Изменено 2 Февраля 2015 пользователем Andrey07071977

[xStream 86]

 

 

Вот и вся суть. Написать комментарий - секундное дело для человека, который этот код написал.

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

Если нетривиальное что-то, можно описать функцию/метод. В общем, все это остается на совести автора так или иначе.

[KD87 715]

Насчет комментариев и self explained кода:

 

-- Функция удаления динамитного ящика из Волка
function delete_dynamite_box(npc, actor)
local dynamite = db.actor:object("dynamite")
if dynamite then
local delete_dynamite = alife():object(dynamite:id())
if delete_dynamite then alife():release(delete_dynamite, true) end
end

Вроде комментарий есть, все понятно, и делает функция ровно то, что описано, но надо бы ее к чертям переписать. Реальный пример. Осталось от предыдущих разработчиков. Понимайте, как хотите, просто ремарка в тему.

[НаноБот 705]

 

 

и делает функция ровно то, что описано

Не понятно, но судя по скрипту удаляет динамит то, из инвентаря актора, а не Волка. А в комментарии написано удаляет из Волка, т.е. из его инвентаря.

[xStream 86]

@KD87, отступы бы существенно помогли. А еще оно действительно делает не то, что написано в комменте

[Malandrinus 611]

@KD87, код прекрасен. Замечательная иллюстрация ситуации "всё плохо". Более того, плохо не в этом коде, который только верхушка айсберга. Всё плохо там, откуда эта функция вызывается. Отложим в сторону несоответствие комментария и действия, предположим, что на самом деле надо удалить объект из инвентаря актора. Итак, здесь две проверки. Первая - что инвентарь актора содержит объект с заданной секцией. Здесь на самом деле неопределённость, и требуется знать контекст вызова: для чего и в какой ситуации вызывается эта функция. Сразу можно сказать, что комментарий - шлак, поскольку не объясняет, зачем выполняется действие. Он должен звучать как, скажем, "отдаём динамит" или "чистим инвентарь от динамита". Разница есть, поскольку такие разные варианты будут подразумевать разные проверки.
"Отдаём динамит" подразумевает, что функция вызывается в конце диалога сдачи квеста. Динамит при этом обязан быть в инвентаре, а иначе просто недоступна будет ветка. Тогда и проверка на самом деле не нужна. Если же мы имеем этот вариант, и такая ошибка вдруг возникает, то значит криво сделан наш квест. На этот случай надо ставить отладочную проверку:
ASSERT(dynamite, "[delete_dynamite_box] no 'dynamite' object in actor's inventory")
Если вывалится, то надо не в эту функцию заплатки ставить, а чинить структуру квеста или диалогов.
Вариант "чистим" может скажем вызываться по любому окончанию квеста, провальному или нет, для очистки инвентаря (странный вариант, ну вдруг нам так захотелось). Тогда и впрямь возможна ситуация, что динамит есть/динамита нет. Тогда и проверка нужна, причём именно такая: если нет, то ничего не делаем. Это на самом деле был бы плохой дизайн, поскольку одна функция совмещает в себе две.
Следующая проверка на наличие серверного объекта - вот это реальное зло. А как вообще может так случиться, что онлайновый объект есть, а серверного нет? Такого в принципе быть не должно. Ну один вариант есть - если мы умудрились вызвать эту функцию два раза подряд. И опять, не в этой функции должны быть такие уродские заплатки, а вызывающий код надо чинить. Здесь же опять надо поставить отладочную проверку:
ASSERT(delete_dynamite, "[delete_dynamite_box] found no server object for dynamite! Check the caller code.")

Итого, допустим функция "отдаёт" динамит Волку. Тогда, включая внятные комментарии, я бы написал так:

-- Отдать динамит Волку (удалить из инвентаря)
function delete_dynamite_box(npc, actor)
    local dynamite = db.actor:object("dynamite")
    ASSERT(dynamite, "[delete_dynamite_box] no 'dynamite' object in actor's inventory")
    local sdynamite = alife():object(dynamite:id())
    ASSERT(sdynamite, "[delete_dynamite_box] found no server object for dynamite! Check the caller code.")
    alife():release(sdynamite, true)
end

А в релизе и вовсе можно написать просто

-- Отдать динамит Волку (удалить из инвентаря)
function delete_dynamite_box(npc, actor)
    local sim = alife()
    sim:release(sim:object(db.actor:object("dynamite"):id()), true)
end

Ведь в релизе уже не должно возникнуть ситуации, когда этот код может сбоить.

Изменено 3 Февраля 2015 пользователем malandrinus

[Dennis_Chikin 3 658]

что онлайновый объект есть, а серверного нет? Такого в принципе быть не должно.

Есть. Во-первых, болт, серверного объекта просто не имеющий.

Во-вторых, неоднократно встречался с ситуацией, когда подряд отрабатывают два скприпта, удаляющих один и тот же объект, и вот здесь любые манипуляции кроме получения id от игрового дают набор самых забавных эффектов.

 

А, да, в выкладывавшемся мной недавно, как тут изящно изволил выразиться один из пользователей, "дерьмокоде" имеет место быть иллюстрация с игровым объектом, полученным через drop-коллбэк.

local obj = alife():object( id )

if obj then

if obj.parent_id ~= 65535 then -- переложили в тайник/отдали

else -- выбросили

end

else -- удален

end

 

Ну, правда, случай несколько иной, но настораживающий. Допускаю, что через actor:object() в определенных ситуациях получить тоже вполне можно.

 

Наконец, странные манипуляции с патронами при смерти непися, попытка удалить которые из death_manager тоже в общем-то заканчивается непредсказуемо.

[boryan67 37]

Именно для упрощения сопровождения я очень густо комментирую свой код. Почти буквально каждую строку. Я делаю это не для кого-то, а для себя. Здорово упрощает жизнь неделю спустя, когда смотришь на свою функцию, как баран на новые ворота, и думаешь: "Это я что-ли написал? А это вообще что?"

Улыбнуло, ибо вспомнил, как еще пару-тройку лет назад я писал для микроконтроллеров программы на Ассемблере и С на уровне железа. Поначалу просто балдел возможностям С с помощью хитромудрых сочетаний всяких "зябликов" собрать в одну строку весьма объемный кусок кода с чтением/записью регистров и портов ввода/вывода с одновременными сдвигами, математикой и наложением масок... Но какой это незабываемый кайф был, спустя год по требованию заказчика модернизировать программы... Слов нет, одни междометия - на кой ... я так это всё извратил в свое время? Это был просто вынос собственного мозга расшифровать эти "древние письмена". И самое обидное - материть пришлось самого себя за такие изысканные решения и стиль написания.

[Malandrinus 611]

Есть. Во-первых, болт, серверного объекта просто не имеющий.

Но здесь то не болт. И не событие дропа, о чём я ниже ещё скажу. В этом конкретном случае отсутствие серверного объекта может означать только косяк в месте вызова, который надо лечить, а не маскировать такими заплатками. Чего мы добились, поставив те проверки? Что нет вылета? Но проблему то не решили. Почему функция вызвалась второй раз? Почему в инвентаре не оказалось нужного объекта? Мы может и заткнём эти косяки здесь, но источник проблем мы так не убираем, и это может сказаться где-то ещё. Вообще странно, что надо доказывать настолько банальную истину: лечить надо болезнь, а не симптомы.

 

А вот это

подряд отрабатывают два скприпта, удаляющих один и тот же объект,

и это

игровым объектом, полученным через drop-коллбэк.

Вот типичная ситуация. Биндер и в нём колбек на дроп, или юзание, или ещё какую операцию с предметом в инвентаре. Наблюдаем нечто вроде:

 

function actor_binder:on_item_drop(obj)
    ...
    supermod1.item_drop(obj)
    supermod2.drop_item(obj)
    ...
    supermod9001.on_item_drop(obj) -- здесь взяли и удалили предмет
    ...
    supermod100500.on_drop(obj)
    ...
end

 

Как ты верно заметил, какой-то из этих вызовов может сделать с предметом что-то нехорошее, удалить к примеру. Все последующие вызовы после этого будут иметь дело с ситуацией наличия клиентского и отсутствия серверного объекта. По этой причине во всех таких вызовах надо ставить проверку на серверный объект. Нельзя не признать, что при такой организации проверка эта нужна.

 

А теперь как это решается при использовании ивентов (на примере моей системы с некоторыми упрощениями).

 

function actor_binder:on_item_drop(obj)
    self.am:call("on_drop", obj, sim:object(obj:id())) -- посылаем сигнал (генерим событие)
end

 

Обращаю внимание, нам больше не потребуется менять ни строчки в биндере актора. Теперь в модуле, где мы хотим использовать это событие, надо написать так:

 

function attach(sm)
    -- подпишем функцию на событие
    sm:subscribe({signal = "on_drop", fun = this.on_drop_item})
end
 
function on_drop(item, sitem) -- эта функция вызовется автоматически
    -- делаем здесь что-то с предметом
end

Аналогично в системе xStream:

 

-- в биндере
function actor_binder:on_item_drop(obj)
    event("actor_item_drop"):trigger({what = obj}) -- посылаем сигнал (генерим событие)
end

-- в модуле
function init()
    event("actor_item_drop"):register(on_drop_item)
end
function on_drop(e) -- эта функция вызовется автоматически
    -- делаем здесь что-то с предметом, передаваемым через поле таблицы e.what
end

Теперь, как решается проблема убитых серверных объектов. На один сигнал можно подписать много разных функций из многих разных модулей (ещё раз обращаю внимание, что при этом не требуется менять ни строчки в самом биндере). При срабатывании события дропа все эти функции вызываются последовательно одна за другой. Допустим, одна из них удаляет предмет. От этого момента вызовы оставшихся в очереди колбэков не имеют смысла, поэтому тот колбэк, что удаляет предмет, должен просигналить, что он является последним в цепочке вызовов. В моей системе такой кольэк должен вернуть true, а в системе xStream вызвать e:stop(). После этого все остальные колбеки для этого конкретного сработавшего события не вызовутся. К примеру реализация какого-либо используемого из инвентаря предмета могла бы выглядеть так:

 

function on_use(item, sitem)
    if is_item_of_some_type(item) then -- определяем тип предмета
        -- здесь делаем что-то, что хотели сделать по юзанию этого предмета
        -- и удаляем его, поскольку он одноразовый
        alife():release(sitem, true)
        return true -- сигналим, что остальным колбэкам срабатывать не надо
    end
end

Как видим, правильная организация кода исключает необходимость лишних проверок.

 

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

 

У ивентов/событий есть и другие плюшки: централизованная отладка зависших колбэков, возможность динамического отписывания для экономии и т.п. Однако главное, что я выше уже говорил - для подписывания не нужно лезть в биндер (или вообще туда, откуда генерится событие). Код становится намного более модульным, существенно снижаются затраты на интеграцию и отладку новых модулей.

 

 

[Dennis_Chikin 3 658]

"лечить надо болезнь, а не симптомы." - На самом деле не надо доказывать. Самоочевидно.

Просто, есть разные лекарства, ну или даже разные названия для одного и того же лекарства.

И, в общем-то, само лечение сводится к соблюдению определенной дисциплины. Которую можно поддерживать добровольно, либо быть вынужденным/заставлять поддерживать техническими средствами.

 

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

 

Что и как выбрать - вот это интересный вопрос. Подозреваю, что в итоге все равно все будет определяться личными вкусами. Кому-то не лень 100500 оберток написать, и еще охранника с дубиной приставить. Кто-то решил, что будет следовать правилам, которые для себя же однажды и установил. Кто-то выберет нечто среднее.

 

 

Однако, имея дело с неким супермегамодом, слепленным неизвестно кем и неизвестно из чего, иногда приходится и затычками пользоваться. По тому как невозможно сделать ВСЕ СРАЗУ. И вот здесь надо знать, что игровой объект без серверного случиться таки может.

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

Возможно тот динамит каким-нибудь babah.scriptом взрывается. Или в аномалию выкидывается, которая его тут же и удаляет.

---

Подумавши: наверное, это еще и к вопросу о комментировании. И документировании.

 

"supermod9001.on_item_drop(obj) -- здесь взяли и удалили предмет

...

supermod100500.on_drop(obj)"

 

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

 

Опять же "От этого момента вызовы оставшихся в очереди колбэков не имеют смысла, поэтому тот колбэк, что удаляет предмет, должен просигналить, что он является последним в цепочке вызовов." - аналогично, неочевидно, почему именно. Если комментарии это не разъясняют. Либо отдельное описание.

 

P.S. Кстати, а почему не имеют ? Серверного объекта нет, а вызовы вполне могут и иметь.

 

2 xStream: Алилуйя ! А то все анекдот про "гуталина нет" вспоминался.

Изменено 4 Февраля 2015 пользователем Dennis_Chikin

[xStream 86]

@Malandrinus, в догонку к твоему шикарному описанию в контексте сталкера: становится возможным быстрое манипулирование модулями (что самоочевидно вытекает из концепции) - то, что столько непонимания вызывало в моих аи-схемах, например, - достаточно просто удалить или переименовать модуль, или закомментировать его подключение. (Кстати, поэтому я назвала систему песочницей - изоляция модулей и возможность "играться" )

Эта фича позволяет методом бинарного поиска быстро определить сбойный модуль, если что не так. Хотя это самый жесткач - система построена так, что при подключении модуля может дать предварительный лог, если модуль не подключился - мол айайай, проверьте синтаксис. Короче говоря: это еще и средство отладки (в нагрузку к проверке зависаний и прочему).

@Dennis_Chikin, собственно речь о том и была - что хорошо бы людям прививать культуру кода, а не копипастить (вот Саша, к примеру жаловался, что так и не смог изжить АМКшный подход к таймерам и событиям) - все просто привыкли, а обучаться и лень и некому. Вот в ОГСЕ научились и вышло неплохо.

 

 

аналогично, неочевидно, почему именно. Если комментарии это не разъясняют. Либо отдельное описание.

Собственно к системам есть довольно подробные мануалы с десяток пунктов с примерами (да и самих методов у тех же сигналов 3-4 штуки - вот и весь интерфейс). Все очень просто. Другим колбекам даже не надо знать, что объект удалили - они просто не сработают.

Изменено 3 Февраля 2015 пользователем xStream

[abramcumner 1 023]

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

А не получится так, что за ним шел коллбек, который у себя вел учет предметов в инвентаре(на on_take добавлял в словарик, на on_drop удалял из словарика). А тут до него коллбек не дошел и предмет так и остался висеть не удаленным. Ну и наоборот.

[Malandrinus 611]

почему не имеют ? Серверного объекта нет, а вызовы вполне могут и иметь.

Ну нет! Сам же говорил насчёт дисциплины, а вот это допущение - нарушение всех возможных норм безопасности. Мы удалили объект - всё, от этого момента его нет. Огрызок от объекта в виде его онлайновой части - это проклятие движка. Никаких дел с ним иметь нельзя, чего мы и добиваемся, предотвращая все последующие вызовы.

 

На самом деле - вот эта ограничительная мера при использовании ивентов - это тоже элемент дисциплины. Я же не могу никак заставить следовать этому требованию всех пользователей системы событий. Если разработчик удалит объект, а return true не поставит, то всем остальным придётся по старинке проверять на наличие серверного объекта. Разница однако в том, что при старом подходе все обязаны были это делать за неимением другого выхода, а при использовании ивентов/событий есть альтернатива: вместо того, чтобы всем проверять, проверить только одному. Опять же - это в итоге экономит время разработки.

 

 

 

становится возможным быстрое манипулирование модулями

В точку! Я так использую свою систему для создания отладочных модулей. Такой модуль - это вообще отдельный файл, кидаешь его в папку со скриптами и он подключается сам (без единой строчки кода где-либо ещё). Страшно удобно. Можно сделать какой-то инструмент отладки, мониторинга каких-то объектов и т.п., который потом можно убрать, просто удалив файл. У нас так сделаны читовый телепорт, редактор/визуализатор зон и т.п. Я делал тулзу для настройки прожекторов: наводишь на прожектор, нажимаешь сочетание клавиш, объект запоминается, потом наводишь куда прожектор должен смотреть, нажимаешь другую клавишу - прожектор поворачивается, а в лог идёт точка направления для настройки логики. В репозитарий я этот файл не включаю, поэтому в игре его нет. При этом, нигде он не прописан, так что его невключение ничего не рушит.

 

@abramcumner, хороший вопрос. Может получиться, согласен. Однако колбеки вообще не должны пересекаться "по интересам", т.е. по множествам предметов с которыми они работают. Т.е. если какой-то колбек занимается коллекционированием предметов с каким-то целями, то он должен при этом фильтровать их дабы не затесались "не его". Ну и я бы старался избегать практики ведения коллекций предметов, особенно в инвентаре.

 

=) На самом деле всегда можно найти способ сломать любую, даже самую надёжную, систему. В Сталкере со всеми его заморочками для этого требуются лишь минимальные усилия. Надёжность в таком контексте - вещь сугубо относительная. Я думаю, что использование ивентов/событий позволяет строить в целом более надёжную систему, нежели традиционный подход.

Изменено 3 Февраля 2015 пользователем malandrinus

[Карлан 1 049]

@abramcumner, в таких случаях перед удалением можно дропать, как вариант.