Механика варнингов #2094

daxsc · 2020-04-18T18:19:32Z

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

close #1247

Pull Request полностью завершен, мне не нужна помощь чтобы его закончить.
Я внимательно прочитал все свои изменения и багов в них не нашел.
Я запускал сервер со своими изменениями локально и все протестировал.
Я ознакомился c Guide to Contribute.

Epicus7 · 2020-04-19T00:26:19Z

code/_helpers/warnings.dm

+	if(!type)
+		return


Если здесь type == null, то скорее всего где-то серьезная ошибка, которая может сломать загрузку/сохранение истории варнингов, что может привести, например, к спаму игроков каждый раунд.

Мне кажется здесь целесообразно использовать ASSERT, чтобы в случае такой ошибки мы получали рантайм и смогли заметить её как можно раньше.

Suggested change

if(!type)

return

ASSERT(type)

Подробнее можно почитать в нашем Guide to Contribute

Тоже самое в других местах с похожими проверками. Если проверяемое условие НИКОГДА не должно выполняться, то нужно использовать ASSERT, чтобы ловить рантаймы, когда условие все-таки выполнится.

Epicus7 · 2020-04-19T00:35:53Z

code/modules/client/preference_setup/occupation/occupation.dm

+	if(user.client.check_warnings("heads") && !(user.client.key in GLOB.newcomers_head_warned))
+		if(role == "Captain" || role == "Head of Personnel" || role == "Chief Engineer" || role == "Chief Medical Officer" || role == "Research Director" || role == "Head of Security")
+			user.client.warn_player("heads", "window=Warning;size=440x300;can_resize=0;can_minimize=0")
+			var/answer = alert(user, "Are you sure you want to play as head of department?",, "Yes", "No")


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

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

В любом случае, я думаю алерт этот не нужен.

Epicus7 · 2020-04-19T00:48:45Z

code/modules/client/preferences.dm

+	if(!(C.key in GLOB.newcomers_warned) && C.check_warnings("newcommers"))
+		C.warn_player("newcommers", "window=Warning;size=360x240;can_resize=0;can_minimize=0")
+		GLOB.newcomers_warned.Add(C.key)
+		C.warned("newcommers")


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

client.show_warning(NEWCOMMERS_WARNING_TYPE)

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

Для этого, очевидно, надо будет еще где-то определить

#define NEWCOMMERS_WARNING_TYPE "newcommers"

Кстати сделав такие дефайны, искать по коду будет гораздо проще. "NEWCOMMERS_WARNING_TYPE" встречается реже, чем "newcommers", а "HEADS_WARNING_TYPE" будет проще найти, чем "heads".

Epicus7 · 2020-04-19T01:35:04Z

code/_helpers/warnings.dm

+		return
+	var/datum/warn/W = new()
+	var/message = file(W.warnings_path+type+".html")
+	show_browser(src, message, options)


Комментарий, касательно всей архитектуры в целом.

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

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

Всего этого проще добиться, если сделать глобальную ПОДСИСТЕМУ "warnings", которая будет загружать на старте sav-файл, хранить историю варнингов в себе, а также хранить в себе все твои глобальные списки и прочую информацию, которая тебе нужна.

Подсистема реализуется примерно так:

SUBSYSTEM_DEF(warnings) name = "Warnings" init_order = SS_INIT_WARNINGS flags = SS_NO_FIRE // это означает, что подсистему не нужно вызывать каждый тик - она просто хранит информацию, а вызывается напрямую через `SSwarnings.func()` var/list/shown_warnings = new // все предупреждения когда либо показанные игрокам. В листе лежат типы варнингов, каждому типу соответствует список сикеев. var/list/shown_warnings_this_round = new // предупреждения, показанные в этом раунде. Тот же формат /datum/controller/subsystem/warnings/Initialize(timeofday) var/savefile/S = new(WARNINGS_DATA_PATH) for (var/type in S) S[type] >> shown_warnings[type] for (var/type in WARNINGS_ALL_TYPES) shown_warnings_this_round[type] = list() // изначально пустой /datum/controller/subsystem/warnings/proc/show_warning(client/client, type) ASSERT(client) if (!(client.ckey in shown_warnings_this_round[type]) && !(client.ckey in shown_warnings[type])) var/message = file(WARNINGS_TEMPLATES_PATH+type+".html") show_browser(client, message, "window=Warning;size=480x320") shown_warnings_this_round[type] += client.ckey shown_warnings[type] += client.ckey __save_shown_warnings() /datum/controller/subsystem/warnings/proc/__save_shown_warnings() // __ указывают, что функция предназначена только для внутреннего использования fdel(file(WARNINGS_DATA_PATH)) var/savefile/S = new(WARNINGS_DATA_PATH) for(var/type in shown_warnings) S[I] << shown_warnings[type]

Где-то в дефайнах определяем дефайны. WARNINGS_ALL_TYPES нужно по аналогии сделать:

#define PATREON_NONE "none" #define PATREON_CARGO "cargo" #define PATREON_ENGINEER "engineer" #define PATREON_SCIENTIST "scientist" #define PATREON_HOS "hos" #define PATREON_CAPTAIN "captain" #define PATREON_WIZARD "wizard" #define PATREON_CULTIST "cultist" #define PATREON_ASSISTANT "assistant" #define PATREON_ALL_TIERS list(\ PATREON_NONE, PATREON_CARGO, PATREON_ENGINEER, \ PATREON_SCIENTIST, PATREON_HOS, PATREON_CAPTAIN, \ PATREON_WIZARD, PATREON_CULTIST, PATREON_ASSISTANT)

В коде, который использует варнинги, пользуемся так:

if (do_we_need_warn()) SSwarnings.show_warning(mob.client, WARNING_TYPE)

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

Epicus7 · 2020-04-19T01:38:52Z

Я ознакомился c Guide to Contribute.

Ну ты все-таки прочитай, там много полезных и интересных вещей написано

daxsc · 2020-04-19T11:17:55Z

@Epicus7 сделал как субсистему.

PlinHost

"newcomers" пишется с одной m, пофиксите плс, читать больно.

daxsc · 2020-04-19T17:12:43Z

"newcomers" пишется с одной m, пофиксите плс, читать больно.

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

Epicus7

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

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

Epicus7 · 2020-04-19T17:31:02Z

code/_helpers/warnings.dm

+	ASSERT(C)
+	ASSERT(type)
+	ASSERT(options)
+	if (!(C.key in shown_warnings_this_round[type]) && (shown_warnings[type][C.key] < 3))


3 - "магическое" число, такое лучше через define определять.

#define WARNINGS_REPEAT_TIMES 3

Epicus7 · 2020-04-19T17:34:20Z

code/modules/client/preference_setup/occupation/occupation.dm

@@ -272,6 +272,8 @@
 			pref.job_low |= job.title
 		return 1

+	if(role == "Captain" || role == "Head of Personnel" || role == "Chief Engineer" || role == "Chief Medical Officer" || role == "Research Director" || role == "Head of Security")
+		SSwarnings.show_warning(user.client, WARNINGS_HEADS, "window=Warning;size=440x300;can_resize=0;can_minimize=0")


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

Алсо окно, конечно, получается не слишком красивое. Шаблоны на HTML довольно бедные, не переиспользуют готовые стили, например, от NanoUI. Еще можно было бы подумать, как избавиться от тайтлбара в этих окошках вообще.

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

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

Дефолтный размер это неоч, в идеале бы динамически его считать, в зависимости от контента. Но я сам сейчас не знаю как это сделать.

Можно в самом хтмл файле давать размер body а потом в бйонде парсить его для окна.

Так это тоже самое, все равно размеры руками указывать.

newcommers > newcomers defined WARNINGS_REPEAT_TIMES

daxsc added 2 commits April 18, 2020 19:46

warnings system

57d981c

Merge branch 'dev' of https://github.com/ChaoticOnyx/OnyxBay into dev

0bb8161

daxsc requested a review from a team as a code owner April 18, 2020 18:19

daxsc added 3 commits April 18, 2020 21:22

Update .gitignore

76ecf86

Update .gitignore

ce58ec3

templates

01aa5cc

Epicus7 requested changes Apr 19, 2020

View reviewed changes

warnings remade

b75ebf6

PlinHost reviewed Apr 19, 2020

View reviewed changes

Epicus7 previously approved these changes Apr 19, 2020

View reviewed changes

small fix

d038d6e

newcommers > newcomers defined WARNINGS_REPEAT_TIMES

daxsc dismissed Epicus7’s stale review via d038d6e April 20, 2020 13:19

Update baystation12.dme

6a8d49a

daxsc requested review from Epicus7 and PlinHost April 20, 2020 13:28

TermService approved these changes Apr 20, 2020

View reviewed changes

Epicus7 approved these changes Apr 20, 2020

View reviewed changes

Epicus7 merged commit cb41c33 into ChaoticOnyx:dev Apr 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Механика варнингов #2094

Механика варнингов #2094

daxsc commented Apr 18, 2020 •

edited

Loading

Epicus7 Apr 19, 2020

Epicus7 Apr 19, 2020

Epicus7 Apr 19, 2020

Epicus7 Apr 19, 2020

Epicus7 Apr 19, 2020

Epicus7 commented Apr 19, 2020

daxsc commented Apr 19, 2020

PlinHost left a comment

daxsc commented Apr 19, 2020

Epicus7 left a comment

Epicus7 Apr 19, 2020

Epicus7 Apr 19, 2020

daxsc Apr 19, 2020

Epicus7 Apr 19, 2020

daxsc Apr 19, 2020

Epicus7 Apr 19, 2020

Механика варнингов #2094

Механика варнингов #2094

Conversation

daxsc commented Apr 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Epicus7 commented Apr 19, 2020

daxsc commented Apr 19, 2020

PlinHost left a comment

Choose a reason for hiding this comment

daxsc commented Apr 19, 2020

Epicus7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daxsc commented Apr 18, 2020 •

edited

Loading