Оптимизация /members

[gambit26]

Писал команду /members. Захотел сделать так, чтобы список игроков сортировался по рангу. В общем в силу скромных знаний и любви к костылям, получилось нечто:


CMD:members(playerid, params[])
{
new string[1000];
if(PlayerInfo[playerid][pMember] > 0)
{
new i;
format(string, sizeof(string), ""COL_YELLOW"Сотрудники Online:");
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 10) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 9) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 8) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 7) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 6) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 5) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 4) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 3) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 2) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
foreach(i : Player)
{
if(!IsAValidPlayer(i)) continue;
if(PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember]) continue;
if(PlayerInfo[i][pRank] != 1) continue;
format(string, sizeof(string), "%s\n{FFFFFF} %s | ID: %d | %s [%d]",string,PlayerInfo[i][pName],i,PlayerInfo[i][pRankName],PlayerInfo[i][pRank]);
}
return SPD(playerid, 0, DSM, "{00BFFF}Работники", string, "Закрыть", "");
}
return SCM(playerid, COLOR_GREY, " Вам недоступна эта команда");
}

Разумеется, вопрос: что можно с этим сделать?

[AavaiableRp]

new i; убери, в foreach объявлена и так перменная...

Зачем столько раз использовать цыкл
foreach если можно один раз а там через
switch(Player[i][Rang])


format(string, sizeof(string), ""COL_YELLOW"Сотрудники Online:");
замени на

SendClientMessage(playerid, COL_YELLOW, "Сотрудники Online:");


new string[1000];

это очень большой массив.....

[VVWVV]

У вас в коде очень большое количество циклов, их можно заменить на один. Так же, очень много ненужных проверок. Советую прочитать все темы про оптимизацию кода(Вы сможете найти их на данном сайте, а так же на официальном сайте: SA:MP).

Вот ваша оптимизированная команда:

CMD:members(playerid, params[])
{
#define MAX_LINES (10)
if (PlayerInfo[playerid][pMember] == 0)
return SCM(playerid, COLOR_GREY, !"Вам недоступна эта команда");
static const
sFrmStr[] = "{FFFFFF} %s | ID: %d | %s[%d]\n";
const
iFStrLen = sizeof sFrmStr + (-2 * 4) + 5 + MAX_PLAYER_NAME * 2;
new
_sBuffStr[iFStrLen],
sOutt[iFStrLen * MAX_LINES] = "Сотрудники Online:\n";
#undef MAX_LINES
foreach(new i: Player)
{
if (!IsAValidPlayer(i) || PlayerInfo[i][pMember] != PlayerInfo[playerid][pMember])
continue;
format(_sBuffStr, sizeof _sBuffStr, sFrmStr, PlayerInfo[i][pName], i, PlayerInfo[i][pRankName], PlayerInfo[i][pRank]);
strcat(sOutt, _sBuffStr);
}
return SPD(playerid, 0, DSM, !"{00BFFF}Работники", _sBuffStr, !"Закрыть", !"");
}
Вы можете изменить количество выводимых строк, заменив значение костанты «MAX_LINES».
Так же советую вам, не записывать название рангов в массив с игроком.

[Daniel_Cortez]

1. Зачем использовать format, если вы не форматируете текст, а только копируете его?
2. Избавьтесь от китайского кода (http://lurkmore.to/%D0%98%D0%BD%D0%B4%D1%83%D1%81%D1%81%D0%BA%D0%B8%D0%B9_%D0%BA%D0%BE%D0%B4#K.D0.B8.D1.82.D0.B0.D0.B9.D1.81.D0.BA.D0.B8.D0.B9_.D0.BA.D0.BE.D0.B4) и поместите foreach в ещё один цикл, в котором будут перебираться ранги от 10 до 1.
3. Вы храните название ранга в аккаунте?


PlayerInfo[i][pRankName]
Во-первых, достаточно создать глобальную таблицу с названиями рангов для всех организаций, а в аккаунте хранить только номер ранга.
Во-вторых, что вы будете делать, если захотите переименовать какой-нибудь ранг? Обшаривать не только мод, но и каждый аккаунт и заменять все совпадения?
4. Вместо того, чтобы 9 раз проверять подключение каждого игрока, статус логина и принадлежность к организации, можно сделать массив для ID игроков, один раз пройтись по всем игрокам (foreach), сохранить нужные ID в массив и уже их перебирать 9 раз, но уже с помощью цикла for. Можно создать массив из MAX_PLAYERS ячеек, это будет самый простой способ (и самый быдлокодерский, т.к. при 1000 игроках таким массивом будет использоваться 1/4 стекового пространства, а ведь ещё есть массив string!), но лучше будет подсчитать, сколько строк ("%s\n{FFFFFF} ...", с учётом минимальных размеров форматируемых значений) вместится в массив string - именно столько ячеек достаточно будет под массив для ID нужных игроков (сложно, но можно).



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




new i; убери, в foreach объявлена и так перменная...

Зачем столько раз использовать цыкл
foreach если можно один раз а там через
switch(Player[i][Rang])
Так и было задумано. Правильнее будет заменить вложенными циклами [2].




format(string, sizeof(string), ""COL_YELLOW"Сотрудники Online:");
замени на

SendClientMessage(playerid, COL_YELLOW, "Сотрудники Online:");
Эта строка выводится в диалог...




new string[1000];

это очень большой массив.....
Размер вполне оправдан, т.к. игроков в организации может оказаться много. Не факт, что ещё хватит 1000.

[gambit26]

1. Зачем использовать format, если вы не форматируете текст, а только копируете его?
2. Избавьтесь от китайского кода (http://lurkmore.to/%D0%98%D0%BD%D0%B4%D1%83%D1%81%D1%81%D0%BA%D0%B8%D0%B9_%D0%BA%D0%BE%D0%B4#K.D0.B8.D1.82.D0.B0.D0.B9.D1.81.D0.BA.D0.B8.D0.B9_.D0.BA.D0.BE.D0.B4) и поместите foreach в ещё один цикл, в котором будут перебираться ранги от 10 до 1.
3. Вы храните название ранга в аккаунте?

Во-первых, достаточно создать глобальную таблицу с названиями рангов для всех организаций, а в аккаунте хранить только номер ранга.
Во-вторых, что вы будете делать, если захотите переименовать какой-нибудь ранг? Обшаривать не только мод, но и каждый аккаунт и заменять все совпадения?
4. Вместо того, чтобы 9 раз проверять подключение каждого игрока, статус логина и принадлежность к организации, можно сделать массив для ID игроков, один раз пройтись по всем игрокам (foreach), сохранить нужные ID в массив и уже их перебирать 9 раз, но уже с помощью цикла for. Можно создать массив из MAX_PLAYERS ячеек, это будет самый простой способ (и самый быдлокодерский, т.к. при 1000 игроках таким массивом будет использоваться 1/4 стекового пространства, а ведь ещё есть массив string!), но лучше будет подсчитать, сколько строк ("%s\n{FFFFFF} ...", с учётом минимальных размеров форматируемых значений) вместится в массив string - именно столько ячеек достаточно будет под массив для ID нужных игроков (сложно, но можно).

1. Я ведь добавляю новый к существующему. Первая переменная в format стоит string. Я так понял, что это неправильно, но не пойму, как тогда следует поступить.
2. Не думал, что "цикл в цикле" будет работать.
3. У меня все названия рангов хранятся в массивах. При, допустим, коннекте, для игрока находится нужный ранг в массиве и записывается в PlayerInfo[playerid][pRankName];
4. Как пройтись по игрокам я, слава богу, понимаю. Но как нужные ID сохранить в массив?

[Daniel_Cortez]

1. Я ведь добавляю новый к существующему. Первая переменная в format стоит string. Я так понял, что это неправильно, но не пойму, как тогда следует поступить.
Забыл сказать, это относилось только к 1-му использованию format.


format(string, sizeof(string), ""COL_YELLOW"Сотрудники Online:");



2. Не думал, что "цикл в цикле" будет работать.
Почему бы и нет? Компилятором Pawn поддерживается до 24 вложенных друг в друга циклов (этот лимит можно расширить, покопавшись в исходниках компилятора - нужно лишь увеличить размер wqTABSZ в sc.h).



3. У меня все названия рангов хранятся в массивах. При, допустим, коннекте, для игрока находится нужный ранг в массиве и записывается в PlayerInfo[playerid][pRankName];
Можно, но нафига? Если у вас уже есть таблица названий рангов, зачем плодить лишние копии этих названий в каждом аккаунте?
У игрока есть номер ранга? Есть. Чтобы узнать название ранга, достаточно взять ID организации и номер ранга в аккаунте и по ним найти название в таблице. Сложите 2 и 2.



4. Как пройтись по игрокам я, слава богу, понимаю. Но как нужные ID сохранить в массив?
Я не зря сделал этот пункт самым последним, т.к. он сложен в реализации.
Если сделать массив на 1000 (MAX_PLAYERS) ячеек, это будет явным расточительством, т.к. данные о стольких игроках не влезут в string.
Как я уже писал, разумнее будет подсчитать макс. кол-во строк и получившееся число взять как размер массива игроков.
Вот то, что получилось у меня:


CMD:members(playerid, params[])
{
new family = PlayerInfo[playerid][pMember];
if (family == 0)
family = PlayerInfo[playerid][pLeader];
if (family == 0)
return SendClientMessage(playerid, COLOR_GREY, !"Вы не состоите ни в одной из банд/мафий/фракций.");
static const fmt_str[] = "\n{FFFFFF}%s | ID: %d | %s[%d]";
// Расчёт размера буфера для форматирования.
const buffer_size = sizeof(fmt_str)
+ (-2 + MAX_PLAYER_NAME)
+ (-2 + 5) // ID игрока, в будущих версиях SA:MP значение MAX_PLAYERS
// может увеличиться вплоть до 65535 (5 символов).
+ (-2 + 20)// Максимальная длина названия ранга - 20 символов (?)
// (если больше или меньше, ставьте своё значение).
+ (-2 + 2);// Номер ранга - не больше 10 (2 символа).
// Расчёт макс. количества строк в диалоге.
const MAX_ENTRIES = 2048 / buffer_size;
// Первая часть строки string будет упакованной, поэтому strcat
// будет автоматически упаковывать новые строки при добавлении их к string.
// Благодаря этому в string можно будет вместить в 4 раза больше текста.
static const first_str[] = !"Сотрудники Online:";
new string[buffer_size * MAX_ENTRIES];
string = first_str;
new string_len = sizeof(first_str) - 1;
new buffer[buffer_size];
// Проходим по всем игрокам и "запоминаем" нужных.
new players[MAX_ENTRIES], players_count = 0;
foreach(new i: Player)
{
//if (!IsAValidPlayer(i))
if (0 == gPlayerLogged[i])
continue;
if (PlayerInfo[i][pMember] != family)
continue;
players[players_count++] = i;
}
// Теперь в цикле проверяем ранги от 10 до 1.
if (players_count == 0)
goto show_dialog;
for (new rank = 11, x, i; --rank != 0;)
{
// Проходим по всем кэшированным игрокам.
for (x = -1; ++x < players_count;)
{
// Если у игрока искомый ранг - добавляем новую строку в диалог.
if ((i = players[x]) != rank)
continue;
format(
buffer, sizeof(string), fmt_str,
//PlayerInfo[i][pName], i,
//PlayerInfo[i][pRankName], PlayerInfo[i][pRank]
buffer, i, RankName(playerid), PlayerInfo[i][pRank]
);
// Добавить сформатированный результат к строке string
// и подсчитать её длину после добавления.
string_len += strcat(string, buffer);
// Если в организации много игроков и в string не хватает
// свободного места для новой строки - переходим к показу диалога.
if (string_len + sizeof(buffer) < sizeof(string))
continue;
goto show_dialog;
}
}
show_dialog:
//return SPD(playerid, 0, DSM, !"{00BFFF}Работники", string, !"Закрыть", !"");
return ShowPlayerDialog(
playerid, 0, DIALOG_STYLE_MSGBOX,
!"{00BFFF}Работники", string, !"Закрыть", !""
);
}

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

[StevenH]

Забыл сказать, это относилось только к 1-му использованию format.




Почему бы и нет? Компилятором Pawn поддерживается до 24 вложенных друг в друга циклов (этот лимит можно расширить, покопавшись в исходниках компилятора - нужно лишь увеличить размер wqTABSZ в sc.h).



Можно, но нафига? Если у вас уже есть таблица названий рангов, зачем плодить лишние копии этих названий в каждом аккаунте?
У игрока есть номер ранга? Есть. Чтобы узнать название ранга, достаточно взять ID организации и номер ранга в аккаунте и по ним найти название в таблице. Сложите 2 и 2.



Я не зря сделал этот пункт самым последним, т.к. он сложен в реализации.
Если сделать массив на 1000 (MAX_PLAYERS) ячеек, это будет явным расточительством, т.к. данные о стольких игроках не влезут в string.
Как я уже писал, разумнее будет подсчитать макс. кол-во строк и получившееся число взять как размер массива игроков.
Вот то, что получилось у меня:


CMD:members(playerid, params[])
{
new family = PlayerInfo[playerid][pMember];
if (family == 0)
family = PlayerInfo[playerid][pLeader];
if (family == 0)
return SendClientMessage(playerid, COLOR_GREY, !"Вы не состоите ни в одной из банд/мафий/фракций.");
static const fmt_str[] = "\n{FFFFFF}%s | ID: %d | %s[%d]";
// Расчёт размера буфера для форматирования.
const buffer_size = sizeof(fmt_str)
+ (-2 + MAX_PLAYER_NAME)
+ (-2 + 5) // ID игрока, в будущих версиях SA:MP значение MAX_PLAYERS
// может увеличиться вплоть до 65535 (5 символов).
+ (-2 + 20)// Максимальная длина названия ранга - 20 символов (?)
// (если больше или меньше, ставьте своё значение).
+ (-2 + 2);// Номер ранга - не больше 10 (2 символа).
// Расчёт макс. количества строк в диалоге.
const MAX_ENTRIES = 2048 / buffer_size;
// Первая часть строки string будет упакованной, поэтому strcat
// будет автоматически упаковывать новые строки при добавлении их к string.
// Благодаря этому в string можно будет вместить в 4 раза больше текста.
static const first_str[] = !"Сотрудники Online:";
new string[buffer_size * MAX_ENTRIES];
string = first_str;
new string_len = sizeof(first_str) - 1;
new buffer[buffer_size];
// Проходим по всем игрокам и "запоминаем" нужных.
new players[MAX_ENTRIES], players_count = 0;
foreach(new i: Player)
{
//if (!IsAValidPlayer(i))
if (0 == gPlayerLogged[i])
continue;
if (PlayerInfo[i][pMember] != family)
continue;
players[players_count++] = i;
}
// Теперь в цикле проверяем ранги от 10 до 1.
if (players_count == 0)
goto show_dialog;
for (new rank = 11, x, i; --rank != 0;)
{
// Проходим по всем кэшированным игрокам.
for (x = -1; ++x < players_count;)
{
// Если у игрока искомый ранг - добавляем новую строку в диалог.
if ((i = players[x]) != rank)
continue;
format(
buffer, sizeof(string), fmt_str,
//PlayerInfo[i][pName], i,
//PlayerInfo[i][pRankName], PlayerInfo[i][pRank]
buffer, i, RankName(playerid), PlayerInfo[i][pRank]
);
// Добавить сформатированный результат к строке string
// и подсчитать её длину после добавления.
string_len += strcat(string, buffer);
// Если в организации много игроков и в string не хватает
// свободного места для новой строки - переходим к показу диалога.
if (string_len + sizeof(buffer) < sizeof(string))
continue;
goto show_dialog;
}
}
show_dialog:
//return SPD(playerid, 0, DSM, !"{00BFFF}Работники", string, !"Закрыть", !"");
return ShowPlayerDialog(
playerid, 0, DIALOG_STYLE_MSGBOX,
!"{00BFFF}Работники", string, !"Закрыть", !""
);
}

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

Вау! Вот это код!!

[gambit26]

всё вроде бы понял. кроме этого




const buffer_size = sizeof(fmt_str)
+ (-2 + MAX_PLAYER_NAME)
+ (-2 + 5) // ID игрока, в будущих версиях SA:MP значение MAX_PLAYERS
// может увеличиться вплоть до 65535 (5 символов).
+ (-2 + 20)// Максимальная длина названия ранга - 20 символов (?)
// (если больше или меньше, ставьте своё значение).
+ (-2 + 2);
[/spoiler]



почему -2?

[Desulaid]

static const fmt_str[] = "\n{FFFFFF}%s | ID: %d | %s[%d]";
const buffer_size = sizeof(fmt_str) + (-2 + MAX_PLAYER_NAME) + (-2 + 5) + (-2 + 20) + (-2 + 2);

[gambit26]

static const fmt_str[] = "\n{FFFFFF}%s | ID: %d | %s[%d]";
const buffer_size = sizeof(fmt_str) + (-2 + MAX_PLAYER_NAME) + (-2 + 5) + (-2 + 20) + (-2 + 2);

понял. спасибо вам обоим

[#Vito]

Мой вариант был примерно такой:

new member = PlayerInfo[playerid][pMember], r = 1, string[9], str[1028];
for(new i = 0; i < MAX_PLAYERS; i++)
{
if(PlayerInfo[i][pRank] != r || PlayerInfo[i][pRank] != member) continue;
format(string, sizeof(string), "Rank: %i\n", r);
strcat(str, string);
if(i == MAX_PLAYERS)
{
if(r == MAX_RANK) break;
r++;
i = 0;
}
}
ShowPlayerDialog(playerid, ID_DIALOG, 0, "Сотрудники Online", str, "Закрыть","");
В принципе, тоже самое что и в первом посте, но кода меньше.