script [0/10] BUSINESS SYSTEM

[Lukasz 336]

Dobrý den,

na požádání autora toho scriptu, který naleznete zde na fóru (http://pawno.cz/files/file/29-dynamické-nemovitosti/), jsem se rozhodl sepsat menší kritiku kódu. Uvědomte si prosím, že kritika sama o sobě není nic záporného a je velice významná. Bez ní bychom se nikam neposunuli. Omlouvám se předem všem, koho se tento topic nějak nepříjemně dotkne.

 

Ihned po otevření pwn scriptu nám autor zanechal velice znepokojující zprávu

 

 

DIE IN HELL...N00008

 

 

Ignoroval jsem jeho výhrůžky a odvážně pokračoval ve čtení jeho kódu.

 

Asi hned jako první prohřešek jsem narazil na tento krásný kód.

 

stock PlayerName(playerid)
{
   new name[255];
   GetPlayerName(playerid, name, 255);
   return name;
}

Autor má asi jména delší, než povoluje SAMP (zajímalo by mě, proč zrovna 255)

 

Dále tu máme funkci

stock FormatTime(sekund, bool:exact = false)

předpokládám, že jí nepsal sám autor.. nicméně

 

if(sekund < 60) format(str, sizeof(str), "%i sekund", sekund%60);

Protože zbytek po dělení (sekund / 60) nejsou už v proměnné sekund.. (ironie).

 

 

 

V OnFilterScriptInit narážím na další podivnost

 

for(new i = 0, j = GetPlayerPoolSize(); i <= j; i++)OnPlayerConnect(i);

Autor zde využil "vychytávku" GetPlayerPoolSize, nicméně volá OnPlayerConnect.. V čem je problém?

 

public OnPlayerConnect(playerid) // ***** tu má OnPlayerConnect z jiného FS asi 
{
    VEZENI[playerid] = TextDrawCreate(272.000000, 380.000000, "_");
    TextDrawBackgroundColor(VEZENI[playerid], 255);
    TextDrawFont(VEZENI[playerid], 1);
    TextDrawLetterSize(VEZENI[playerid], 0.500000, 1.000000);
    TextDrawColor(VEZENI[playerid], -1);
    TextDrawSetOutline(VEZENI[playerid], 0);
    TextDrawSetProportional(VEZENI[playerid], 1);
    TextDrawSetShadow(VEZENI[playerid], 1);
    TextDrawSetSelectable(VEZENI[playerid], 0);
    return 1;
}

Problém je asi v tom, že tento textdraw pouze vytváří (a ničí) a nikde nezobrazuje, takže zbytečné pole, zbytečný TD, zbytečný cyklus a zbytečný callback. Tímto se dostáváme do OnFilterScriptExit, kde tento zbytečný TD ničí.

 

 

Další problém nastává hned v dalším callbacku

 

 public OnPlayerKeyStateChange(playerid, newkeys, oldkeys)

if(newkeys & KEY_WALK)
{
     for(new BussID = 0;BussID < sizeof(BusinessInfo);BussID++)
     {
         if(IsPlayerInRangeOfPoint(playerid, 4.0, BusinessInfo[BussID][bEntranceX], BusinessInfo[BussID][bEntranceY], BusinessInfo[BussID][bEntranceZ]))
         {
             new str[256],topstr[64];
             if(smazano[BussID]==1) return SCM(playerid,WAR,"Tato nemovitost bude po restartu smazána");
             {
if(BusinessInfo[BussID][bOwned] == 0)
            {
  format(str,sizeof(str),"Koupit Nemovitost\t\t%i\n Vykrást",BusinessInfo[BussID][bPrice]);


format(topstr,sizeof(topstr),"Nemovitost %s",BusinessInfo[BussID][bName]);
ShowPlayerDialog(playerid,DialogBus1,DIALOG_STYLE_LIST,topstr,str,"Vybrat","Zavřít");
           }
           if(BusinessInfo[BussID][bOwned] == 1) // else nezná?
           {
format(str,sizeof(str),"Prodat Nemovitost\nVybrat výdělky\t\t\t{00ff00}%i{ffffff} $\nPronajmout ochranku\t\tzbyva{00ff00} %s{ffffff}\nVykrást\nInvestovat\nZměnit název Podniku\nProdat nemovitost hráči\nOdkoupit nemovitost",BusinessInfo[BussID][bMoney],FormatTime(BusinessInfo[BussID][bOchranka]-gettime(),true));
format(topstr,sizeof(topstr),"Nemovitost %s",BusinessInfo[BussID][bName]);
ShowPlayerDialog(playerid,DialogBus2,DIALOG_STYLE_LIST,topstr,str,"Vybrat","Zavřít");
}
        }
       }
   }
}
    return 1;

Co tedy autor provádí špatně. Nejprve zjišťuje, u jaké nemovitosti hráč stojí. Mohl použít svojí funkci, kterou používá na stejné případy v jiných částech kódu, stock IsPlayerNearBizEnt(playerid), která vrací id nemovitosti, u které stojí. Dalším problémem je, že IsPlayerInRangeOfPoint není zrovna moc časově nenáročná funkce a autor zde po nalezení id nemovitosti cyklus neukončuje (neznalost break, return ?). Také zde má zbytečnou podmínku, kdy může krásně použít else a nic nekontrolovat dvakrát.

 

Přesouváme se do OnDialogResponse, kde jsem narazil na toto

 

if(dialogid==DialogBus1 && response)

Někomu to může přijít jako vhodné, ale pokud bude response false, neprovede se blok podmínky a provádí se další podmínky. Dostane se to do takové fáze, že se nakonec vrátí 0. Vrácením 0 v OnDialogResponse zapříčiníme to, že se zavolá další OnDialogResponse v jiném scriptu. Zase zcela zbytečné. Zde by bylo vhodné něco ve stylu

 

if(dialogid == DialogBus1)
{
    if(!reponse) return 1;
    zbytek kódu
}

A hned v prvním (i v dalších) dialogu máme tento kód

 

new string[MAX_PLAYER_NAME]; // LOOL
format(string, sizeof(string),PlayerName(playerid));
strmid(BusinessInfo[BussID][bOwner], string, 0, strlen(string), MAX_PLAYER_NAME);

stačilo by, aby PlayerName vracelo pole o velikosti MAX_PLAYER_NAME a mohli bychom udělat něco takového

 

BusinessInfo[BussID][bOwner] = PlayerName(playerid);

zcela zbytečné pole, zbytečný format (to jako wtf) a zbytečný strmid.

 

U dalšího dialogu má autor.

 

if(dialogid==DialogBus3)
{
   if(!response) return 0;
   if(response)

Zase se vrací 0 a volá se další OnDialogResponse

 

Další obdobný problém jako s PlayerName.

 

new string[24];
if (strlen(inputtext) < 5 || strlen(inputtext) > 24) return SCM(playerid, WAR, "Název musí obsahovat 5-24 znaků");
format(string, sizeof(string), inputtext);
strmid(BusinessInfo[BussID][bName], string, 0, strlen(string), 24);

V příkazu "gotobus" se nachází tento kód

 

new BusinessList[2048];

Přičemž do toho pole, ukládá max 300 znaků (?)

 

 

Dalším prohřeškem je tento příkaz

 

CMD:getfreebuss(playerid, params[],help)
{
new pocet[30];
for(new i = 0; i < sizeof(BusinessInfo); i++)
{
if(BusinessInfo[i][bOwned]==0)
{
   if (IsValidDynamicPickup(BusinessInfo[i][bOutsideIcon]))
{
format(pocet,sizeof(pocet),"Volných nemovitostí %i",i); // TIMHLE FAKT NEZÍSKÁ POČET
}
}
   }
        SendClientMessage(playerid,EROR,pocet);
return 1;
}
viz komentář v kódu


Hned v dalším příkazu 
for(new i = 0; i < MAX_PLAYERS; i++) // PlayerPoolSize
{
if (strcmp(BusinessInfo[BussID][bOwner], PlayerName(i), false) == 0)
{
if(IsPlayerConnected(i))
{
   new msg[128];
   format(msg,sizeof(msg),"Tvoje nemovitost %s byla smazána!",BusinessInfo[BussID][bName]);
   SendClientMessage(i,WAR,msg);
}
} // zase break
}

Zde již nevyužívá GetPlayerPoolSize, protože mu asi přišel moc novátorský a znovu "nevyskakuje" z cyklu, když může. A taková perlička, nejprve kontroluje jméno jestli se shoduje a až pak jestli je hráč připojený.

 

 

Následně pak nastavuje timer na public, který v kódu neuvádí, tudíš usuzuji, že to někde okopíroval. Hodnotím 0/10 za snahu. Jsou tam ještě zbytečné proměnné, ale to by tento topic byl o dost delší.

 

Autorem scriptu je náš Vladasavel.

Edited 12. Březen, 2016 by Lukasz

[Scydo 397]

Tímto bych tedy poprosil téma smazat

~> Proč? Luky si s tím nedával námahu, aby se to zase zbytečně mazalo(bohužel se tak stalo(fys)). Mělo to účel, a to odradit jiné začátečníky nebo pokročilé programátory pawn od pomalu fatálních chyb, které se vyskytují v tom scriptu, a dáva příklad, z čeho se neučit. Pokud neumíš snést veřejnou kritiku, tak nezveřejňuji veřejně scripty. K čemu by to pak psal? Protože se nudil? Kdepak.. dokonce to tam i sám zmiňuje proč:

 

 

Uvědomte si prosím, že kritika sama o sobě není nic záporného a je velice významná. Bez ní bychom se nikam neposunuli.

~> Jak jsem říkal, když kašleš na negativní názory, které ti naopak napomáhají k zdokonalování, tak buď zůstaň u něčeho jednoduchého a nebo prostě nepřidávej nic. A výmluvy, že tě to opět nezajímá, nebo, že si se nudil, či, že jsi nám chtěl ukázat něco nového? Pche, to spíše méně teď zajímá ostatní...

 

~> Neříkám, že nedělám chyby. Jak teoretické tak i v praxi... ale alespoň, když už mi to někdo kritizuje, tak ho vyslechnu (samozřejmě, pokud to je uživatel s relevantním IQ, který využívá dostatek mozkové kapacity), a zlepším po případě se poučím a poděkuju, nebo svoje chyby obhájím, že to nejsou chyby ale záměr. Ale nikdy bych ho neposílal do háje s tím, že ať svůj kritický a negativní komentář smaže, po případě ho přesune někam, kde ho sotva někdo čte.

 

PS: Pokud si teď budeš stěžovat, že jsem ti teď tady zkazil náladu a budeš chtít odejít, jen ukážeš svojí slabost ve programování a snaze.

[vladasavel 26]

Tak já se tedy vyjádřím. Vyhružky nebyly myšlený na tebe. Vytrhnul si to z kontextu.

o to Playername jsem ti psal že jsem chybu udělal

Funkce FormatTime je od xhunterx takze pochybuji že tam bude něco špatně

A tady je chvilka kdy asi netušíš proč to tak je nebo si jen hledal pičoviny aby si mě napadnul.

v FS init volám OPC aby se vytvořili PTD hráčům co jsou připojení v případě reloadu. Takže ne OPC není z jiného FS.

Textdrawy slouží k odečítání času když ho ochranka při okrádání zatkne

else znám ale nepoužívám ho. to je taky napadení z tvé strany. Je na mě jak co budu používat a jak budu muj kod psát.

neznalost break: duvod proč bbych musel použít ? ten cyklus jsem nezastavil to vím. Proč taky. server něběží na kalkulačce aby to dělal problém

v if(!response) return 0; nevidím problém 

ne a nic z dalšího FS se nevolá. Tento FS byl stavět na unitu kde je vše krásně chráněný aby se nic neprohodilo. a pokud majitel bude mít problém s tím že se mu budou prohazovat funkce dialogu to je jeho problem ne muj

Podle tebe je zbytečný uplně vše. Jak by si asi zkontoloval jestli hráčovo jmeno se rovná s ownerem ? 

k [2048] server nebezi na kalkulačce aby to dělalo problem ale uznavam je to zbytečně velký pole

Budeš se divit ale kodem jsem počet volných nemovek získal

Ano prvně kontrolují jméno ownera a potom jestli je připojenej.. proč ? z toho duvodu že pokud je připojenej owner tak mu to odešle zprávu.

 

Pokud jsem zapoměl public timeru tak to je tím že nemovky byly hookovaný takže public se nekam zatoulal.

 

Scydo: To co napsal Lukasz není kritika ale  buzerace

já kritiku beru. Nerozčiluji se tu jak  by většina udělal. Ale polovička veci co sepsal je prostě buzerace

[Lukasz 336]

Dekuji za feedback.

Nic mimo kontext neni. Takova zprava tam je a nikde nepisu, ze je mirena na me.

Xhunterx, stejne jako ja, ty a dalsi lide, neni dokonaly a dela chyby. Vlastne jsem mu uz nejake opravil (viz jine forum sekce uzitecny kod nebo tak nejak) a rozumim tomu kodu, neboj.

 

Problem s tim OPC a TD je v tom, ze to pouze vytvaris a nicis. Nikde to nezobrazujes (stejne tak nikde nepouzivas jail).

 

Celkem stupidni argumenty na nepouzivani breaku, else a normalnich velikosti poli. To, ze ty nemas kalkulacku neznamena, ze ostatni ji nemaji. Davas svuj fs ke stazeni a bude zpusobovat zbytecne velkou zatez. Navic i pro ten svuj server bys to mel mit lepe optimalizovane

 

Pak asi vubec nerozumis problemu s return 0 v dialog response a uz vubec ne tomu, ze pokud se hracovo jmeno rovna jmenu zapsanemu, pak je hrac pripojeny a nemusime kontrolovat.

 

Neni zac za ponauceni a snad se dostanes z techto tezkych zacatku v pawn.

[Scydo 397]

Funkce FormatTime je od xhunterx takze pochybuji že tam bude něco špatně

~> Znám xhuntera déle než ty, a ve 99% případech je to zaneprázdněný člověk, který dělá, nebo dokáže dělat na více věcech. Takže, když udělá sem tam nějakou chybičku nebo překlep, tak to není žádná novinka. Uznávám, že jeho znalosti jsou na dost sakrá vysoké úrovni, ale, nemůžu bohužel uznat, že hrdinsky nikdy neudělá chybu.

 

A tady je chvilka kdy asi netušíš proč to tak je nebo si jen hledal pičoviny aby si mě napadnul.

Uvědomte si prosím, že kritika sama o sobě není nic záporného a je velice významná. Bez ní bychom se nikam neposunuli.

----

 

Je na mě jak co budu používat a jak budu muj kod psát.

~> Ano, ale když ho už zveřejníš, tak máme plné právo ho okomentovat, po případě zkritizovat.

 

v if(!response) return 0; nevidím problém

~> Já bohužel ano: "Returning 0 in this callback will pass the dialog to another script in case no matching code were found in your gamemode's [/callback." (zdroj: Wiki)

 

ne a nic z dalšího FS se nevolá. Tento FS byl stavět na unitu kde je vše krásně chráněný aby se nic neprohodilo. a pokud majitel bude mít problém s tím že se mu budou prohazovat funkce dialogu to je jeho problem ne muj

~> Právě naopak. To přesně tvůj problém bude, proč mu bude pak blbnout na serveru. "Tvůj" (ano, má to relevantní důvod, proč je to v uvozovkách) kód, tudíž i tvůj script si dává na server...

 

Scydo: To co napsal Lukasz není kritika ale buzerace

já kritiku beru. Nerozčiluji se tu jak by většina udělal. Ale polovička veci co sepsal je prostě buzerace

Uvědomte si prosím, že kritika sama o sobě není nic záporného a je velice významná. Bez ní bychom se nikam neposunuli.

[vEnd 279]

Určitě to má své mouchy, ale na 0 z 10 to není. Dal bych tomu ty dva tři body.

[ATomas 286]

Vyjadrim se tu. Sice nevim o co jde, nechci se tu pripletat do hadky to si vyreste mezi sebou, chci jen poukazat na jednu vec v OnPlayerKeyStateChange.

To lze zoptimalizovat, pokud si v OnPlayerPickUpPickup podrzime v promene v jakem pickupu je a to potom pouzit v OnPlayerKeyStateChange, pak nebudeme potrebovat zadny cyklus.

 

Dalsi vec

BusinessInfo[BussID][bOwner] = PlayerName(playerid);

toto nelze pouzit, protoze PlayerName vraci string a ten nelze priradit pouhym "="

takze nejak takto:

format(BusinessInfo[BussID][bOwner],MAX_PLAYER_NAME,PlayerName(playerid));

Popripade strmid je na miste u toho bych cekal ze bude rychlejsi nez format.

 

Ale myslim ze tento topic je rozhodne prinosem pro spousti lidi, kteri si mysli ze pawno ovladaji, pritom tam pisou podobne blbosti. Rozhodne je v tom potencial k tomu se neco naucit.

[Lukasz 336]

Každá odpověď a návrhy na vylepšení jsou vítány. 

Následující kód půjde za předpokladu, že PlayerName vrátí pole o stejné, nebo menší velikosti než je bOwner. Kdyby se vrátilo větší, pak nestačí pouhé '=' ale jak říkáš format atd.

BusinessInfo[BussID][bOwner] = PlayerName(playerid);

Je ale trochu k ničemu aby dvě pole, která slouží k uložení stejných informací (o maximální stejné délce), aby měla různé velikosti.

 

To s Pickupem je dobrý nápad.

[ATomas 286]

Napadlo me jeste lepsi reseni:

GetPlayerName(playerid,BusinessInfo[BussID][bOwner],MAX_PLAYER_NAME);

[Lukasz 336]

Jo, to je nejlepší řešení.

++