-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Сохранение расы, пола и скина в базе #466
base: dev
Are you sure you want to change the base?
Conversation
e3627f9
to
c8d590a
Compare
договорились разнести:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вцелом, LGTM. Нужна только небольшая правка по неймингу
mods/lord/lord_classes/init.lua
Outdated
local gender = pmeta:get_string("classes:gender") | ||
if gender == nil or gender == "" then | ||
if table_contains(cache.players, name) then | ||
-- legacy compatibility | ||
gender = cache.players[name][2] | ||
else | ||
gender = races.default[2] | ||
end | ||
pmeta:set_string("classes:gender", gender) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
уж очень похожие 2 куска кода.
напрашивается вынесение в функцию, но можно вынести в отдельную задачу в проект рефакторинга, тем более, что он тут уже давно напрашивается полностью.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Куски короткие и очень понятные, можно и так оставить, не критично
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Совместимость со старым форматом мешает читаемости, может лучше сделать скрипт для переноса существующих данных и убрать легаси код?
mods/lord/lord_classes/init.lua
Outdated
local player = minetest.get_player_by_name(name) | ||
local pmeta = player:get_meta() | ||
|
||
local race = pmeta:get_string("classes:race") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Как насчёт окончательно переименовать classes
в races
(и мод в том числе)?
mods/lord/lord_classes/init.lua
Outdated
local gender = pmeta:get_string("classes:gender") | ||
if gender == nil or gender == "" then | ||
if table_contains(cache.players, name) then | ||
-- legacy compatibility | ||
gender = cache.players[name][2] | ||
else | ||
gender = races.default[2] | ||
end | ||
pmeta:set_string("classes:gender", gender) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Куски короткие и очень понятные, можно и так оставить, не критично
ba6271e
to
d71de60
Compare
mods/lord/lord_classes/init.lua
Outdated
pmeta:set_string("player:race", race_and_gender[1]) | ||
pmeta:set_string("player:gender", race_and_gender[2]) | ||
|
||
-- races.update_player(name, race_and_gender, races.default_skin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
забыл удалить закомментированный код
* Use player meta for storing race, skin, gender, race privs, second chance * Add converter script races.txt -> races.sql
@vladtcvs , сможешь посмотреть что тут было ? |
@vladtcvs , возможно многое поменялось, надо бы вспомнить что тут. |
PR устареыший, требует кучу доработок, но нам нужны скрипты из него. |
Для хранения расы, пола и скина используется метаданные игрока, созраняемые в базе.
Также в lottpotions и hbhunger были вставлены проверки, чтобы не падали