Customizing Goldpc Bonus NPC

[imat1]

Hello guys. Please help me confirm if what I changed from my script could cause any harm or bug/exploit to my server.

Here's the raw NPC script from Initial implementation of the goldpc timer

 

Spoiler

prontera,0,0,0    script    Goldpoint Manager::GOLDPCCAFE    4_F_02,{
    // ID:AMOUNT:PRICE
    setarray .items$[1],
        "25464:1:2", // World_Tour_Ticket 1
        "23919:1:10",  // K_Secret_Key 1
        "23919:11:100",// K_Secret_Key 11
        "23919:33:300";// K_Secret_Key 33

    mes "[Goldpoint Manager]";
    mes "You currently have ^0000ff"+Goldpc_Points+"^000000 points.";
    mes "What reward do you want?";
    next;

    .@menu$ = "View current points";

    for(.@i = 1; .@i < getarraysize(.items$); .@i++) {
        explode(.@array$, .items$[.@i], ":");
        .@cost = atoi(.@array$[2]);
        .@menu$ += ":" + .@cost + " points gift";
        if( Goldpc_Points < .@cost ){
            .@menu$ += " ^ff0000(not enough points)^000000";
        }
    }

    .@s = select(.@menu$)-1;

    if(.@s == 0) {
        mes "[Goldpoint Manager]";
        mes "You currently have ^0000ff"+Goldpc_Points+"^000000 points.";
        close;
    }

    explode(.@array$, .items$[.@s], ":");
    .@itemid = atoi(.@array$[0]);
    .@amount = atoi(.@array$[1]);
    .@cost = atoi(.@array$[2]);

    if(Goldpc_Points < .@cost) {
        mes "[Goldpoint Manager]";
        mes "You have ^0000ff"+Goldpc_Points+"^000000 points remaining.";
        mes "You cannot get the prize with this amount of points.";
        close;
    }

    mes "[Goldpoint Manager]";
    mes "You chose the "+.@cost+" points gift. We will reward you immediately.";
    Goldpc_Points -= .@cost;
    getitem .@itemid,.@amount;
    mes "You have ^0000ff"+Goldpc_Points+"^000000 points remaining.";
    close;
}

 

Here's what i did.

Spoiler

-    shop    hourly_shop    -1,13534:-1.

prontera,0,0,5    script    Hourly Point Manager::GOLDPCCAFE    10380,{
    cutin "ep18_merchant.png",2;
    callshop "hourly_shop",1;
    npcshopattach "hourly_shop";
    end;

OnBuyItem:
    for (.@i = 0; .@i < getarraysize(@bought_nameid); .@i++){
        .@j = inarray(.Shop, @bought_nameid[.@i]);
        .@cost += (.Shop[.@j+1] * @bought_quantity[.@i]);
    }
    mes "[Hourly Point Manager]";
    if (.@cost > Goldpc_Points)
        mes "You don't have enough Hourly Points.";
    else {
        for (.@i = 0; .@i < getarraysize(@bought_nameid); .@i++) {
            getitem @bought_nameid[.@i], @bought_quantity[.@i];
            dispbottom "Purchased " + @bought_quantity[.@i] + "x " + getitemname(@bought_nameid[.@i]) + ".";
        }
        Goldpc_Points -= .@cost;
        mes "Purchased successfully!";
        mes "You have ^0000ff"+Goldpc_Points+"^000000 points remaining.";
    }
    deletearray @bought_quantity, getarraysize(@bought_quantity);
    deletearray @bought_nameid, getarraysize(@bought_nameid);
    close;

OnInit:
    setarray .Shop[0],13534,1,13810,1,14532,1,14606,1,12211,1,7776,25;
    npcshopitem "hourly_shop", 13534,1,13810,1,14532,1,14606,1,12211,1,7776,25;
    end;
}

It's working fine as I have tested. But I don't trust myself. 

Also I refer a lot from rathena's hunting missions.

I'm open for suggestions. Thanks in advance!

 

Note: I don't know C++ (if this is the language we're using here) but I do know javascript because of my job which helps me manage somehow (they're almost the same LOL which surprised me, there's just a lot of difference when it comes to syntax)

[Winterfox]

I didn't see any security issues, but two small things that could be handled a bit smoother.

The first is to add a check if the items bought can be carried by the player.
 

if (!checkweight2(@bought_nameid, @bought_quantity)) {
	mes("Sorry, you can't carry all these items!");
	close3;
}

The second is that if you use shop and the player has not enough zeny even though you want to pay with points, he can't put the item in the buy window. So pointshop is the better option.
 

-	pointshop	hourly_shop	-1,Goldpc_Points,13534:-1.

If you use pointshop you also could omit the whole npcshopattach implementation, if you do not care about the cutin and the message boxes in between.

Putting the two above mentioned changes into you npc it could look like this:

-	pointshop	hourly_shop	-1,Goldpc_Points,13534:-1.

prontera,156,181,5	script	Hourly Point Manager::GOLDPCCAFE	10380,{
    cutin("ep18_merchant.png", 2);
    callshop("hourly_shop", 1);
    npcshopattach("hourly_shop");
    end;

    OnBuyItem:
        mes("[Hourly Point Manager]");
        if (!checkweight2(@bought_nameid, @bought_quantity)) {
            mes("Sorry, you can't carry all these items!");
            close3;
        }
        
        for (.@i = 0; .@i < getarraysize(@bought_nameid); .@i++){
            .@itemIndex = inarray(.Shop, @bought_nameid[.@i]);
            .@costs += (.Shop[.@itemIndex + 1] * @bought_quantity[.@i]);
        }

        if (.@costs > Goldpc_Points) {
            mes("You don't have enough Hourly Points.");
            close3;
        }

        Goldpc_Points -= .@costs;
        for(.@i = 0; .@i < getarraysize(@bought_nameid); .@i++) {
            getitem(@bought_nameid[.@i], @bought_quantity[.@i]);
            dispbottom("Purchased " + @bought_quantity[.@i] + " x " + getitemname(@bought_nameid[.@i]) + ".");
        }

        mes("Purchased successfully!");
        mes("You have ^0000ff" + Goldpc_Points + "^000000 points remaining.");
        close3;

    OnInit:
        setarray(.Shop[0], 13534, 1, 13810, 1, 14532, 1, 14606, 1, 12211, 1, 7776, 25);
        npcshopitem("hourly_shop", 13534, 1, 13810, 1, 14532, 1, 14606, 1, 12211, 1, 7776, 25);
}

 

Edited July 10, 2023 by Winterfox

[imat1]

7 hours ago, Winterfox said:

I didn't see any security issues, but two small things that could be handled a bit smoother.

The first is to add a check if the items bought can be carried by the player.
 

if (!checkweight2(@bought_nameid, @bought_quantity)) {
	mes("Sorry, you can't carry all these items!");
	close3;
}

The second is that if you use shop and the player has not enough zeny even though you want to pay with points, he can't put the item in the buy window. So pointshop is the better option.
 

-	pointshop	hourly_shop	-1,Goldpc_Points,13534:-1.

The reason why I didn't use pointshop is because of this warning message.

It's kind of misleading to me.

Thanks a lot for this though.

7 hours ago, Winterfox said:

 

-	pointshop	hourly_shop	-1,Goldpc_Points,13534:-1.

prontera,156,181,5	script	Hourly Point Manager::GOLDPCCAFE	10380,{
    cutin("ep18_merchant.png", 2);
    callshop("hourly_shop", 1);
    npcshopattach("hourly_shop");
    end;

    OnBuyItem:
        mes("[Hourly Point Manager]");
        if (!checkweight2(@bought_nameid, @bought_quantity)) {
            mes("Sorry, you can't carry all these items!");
            close3;
        }
        
        for (.@i = 0; .@i < getarraysize(@bought_nameid); .@i++){
            .@itemIndex = inarray(.Shop, @bought_nameid[.@i]);
            .@costs += (.Shop[.@itemIndex + 1] * @bought_quantity[.@i]);
        }

        if (.@costs > Goldpc_Points) {
            mes("You don't have enough Hourly Points.");
            close3;
        }

        Goldpc_Points -= .@costs;
        for(.@i = 0; .@i < getarraysize(@bought_nameid); .@i++) {
            getitem(@bought_nameid[.@i], @bought_quantity[.@i]);
            dispbottom("Purchased " + @bought_quantity[.@i] + " x " + getitemname(@bought_nameid[.@i]) + ".");
        }

        mes("Purchased successfully!");
        mes("You have ^0000ff" + Goldpc_Points + "^000000 points remaining.");
        close3;

    OnInit:
        setarray(.Shop[0], 13534, 1, 13810, 1, 14532, 1, 14606, 1, 12211, 1, 7776, 25);
        npcshopitem("hourly_shop", 13534, 1, 13810, 1, 14532, 1, 14606, 1, 12211, 1, 7776, 25);
}

Made me realized a lot of things. Also on OnInit part, I feel so dumb after seeing that I typed the same parameters instead of using for loop. lol

Again, thanks for your suggestions, which I appreciate.

[Outlook]

Like This.?