• There is NO official Otland's Discord server and NO official Otland's server list. The Otland's Staff does not manage any Discord server or server list. Moderators or administrator of any Discord server or server lists have NO connection to the Otland's Staff. Do not get scammed!

Lua My script, how to improve?

Ramirow

Ramirow

Veteran OT User
Joined
Aug 22, 2009
Messages
584
Solutions
15
Reaction score
301
Location
Argentina
YouTube
ramirogrant
Hello guys, I'm trying to learn lua, I don't want to ask or search for scripts, I want to be able to create them myself..So I was looking at the libraries for functions and stuff. I've created an script from scratch.

Can someone analyze the script and tell me if it was done correctly? I would love some feedback on these subject.
Sorry if the script is bad, It's just..My first time lol

This is supposed to:
- Check for an storage value
- Assign the storage at the end
- Give reward depending on player vocation
- If is knight, it gives a different weapon depending on highest skill

I don't know if I used tables correctly or if a var should be used to shorten a function("voc" = getPlayerVocation(cid).)
Code: 
-- Scripted by Ramiro Jesse Grant for Phoenix OTS

local storage = 51222
local voc = getPlayerVocation(cid)
local count = 1
local bag = 2330
local sword = 2
local axe = 3
local club = 1

local items = {
    [1] = {7407, 2445, 7455}, -- Knight Items
    [2] = {8868, 2122}, -- Druid/Sorcerer Items
    [3] = {8858, 2352}    -- Paladin Items
}

function onUse(cid, item, frompos, item2, topos)

    if getPlayerStorageValue(cid, storage) == -1 then
        if voc == 1 or voc == 5 or voc == 9 or voc == 2 or voc == 6 or voc == 10 
                local m = items[2]
                doPlayerAddItem(cid, bag, 1)
                doAddContainerItem(bag, m[1], count)
                doAddContainerItem(bag, m[2], count)
                doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado un "..getItemNameById(bag).." que contiene una "..getItemNameById(m[1]).." y un "..getItemNameById(m[2])..".")
                end
        if voc == 3 or voc == 7 or voc == 11 then
                local p = items[3]
                doPlayerAddItem(cid, bag, 1)
                doAddContainerItem(bag, p[1], count)
                doAddContainerItem(bag, p[2], count)
                doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(bag).." que contiene un "..getItemNameById(p[1]).." y una "..getItemNameById(p[2])..".")
                end
        if voc == 4 or voc == 8 or voc == 12 then
            local k = items[1]
                if getPlayerSkillLevel(cid, sword) > getPlayerSkillLevel(cid, club) and getPlayerSkillLevel(cid, sword) > getPlayerSkillLevel(cid, axe) then
                    doPlayerAddItem(cid, k[1], count)
                    doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una .."getItemNameById(k[1])..".")
                end
                if getPlayerSkillLevel(cid, club) > getPlayerSkillLevel(cid, sword) and getPlayerSkillLevel(cid, club) > getPlayerSkillLevel(cid, axe) then
                    doPlayerAddItem(cid, k[2], count)
                    doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una .."getItemNameById(k[2])..".")
                elseif getPlayerSkillLevel(cid, axe) > getPlayerSkillLevel(cid, sword) and getPlayerSkillLevel(cid, axe) > getPlayerSkillLevel(cid, club) then
                    doPlayerAddItem(cid, k[3], count)
                    doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una .."getItemNameById(k[3])..".")
                    end
            end
        setPlayerStorageValue(cid, storage, 1)
    end
    if(getPlayerStorageValue(cid, storage) > 0) then
    doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "It is empty.")

    end
return TRUE
end
 
Sort by date Sort by votes
You forgot about "then" after "if" :p

Code: 
if voc == 1 or voc == 5 or voc == 9 or voc == 2 or voc == 6 or voc == 10 then

And your decleration of functions in sendMessage is wrong:
Not be:
Code: 
.."function().."
Must be:
Code: 
"..function().."
I fix your code:

PHP: 
-- Scripted by Ramiro Jesse Grant for Phoenix OTS

local storage = 51222
local voc = getPlayerVocation(cid)
local count = 1
local bag = 2330
local sword = 2
local axe = 3
local club = 1

local items = {
  [1] = {7407, 2445, 7455}, -- Knight Items
  [2] = {8868, 2122}, -- Druid/Sorcerer Items
  [3] = {8858, 2352}  -- Paladin Items
}

function onUse(cid, item, frompos, item2, topos)

  if getPlayerStorageValue(cid, storage) == -1 then
  if voc == 1 or voc == 5 or voc == 9 or voc == 2 or voc == 6 or voc == 10 then
  local m = items[2]
  doPlayerAddItem(cid, bag, 1)
  doAddContainerItem(bag, m[1], count)
  doAddContainerItem(bag, m[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado un "..getItemNameById(bag).." que contiene una "..getItemNameById(m[1]).." y un "..getItemNameById(m[2])..".")
  end
  if voc == 3 or voc == 7 or voc == 11 then
  local p = items[3]
  doPlayerAddItem(cid, bag, 1)
  doAddContainerItem(bag, p[1], count)
  doAddContainerItem(bag, p[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(bag).." que contiene un "..getItemNameById(p[1]).." y una "..getItemNameById(p[2])..".")
  end
  if voc == 4 or voc == 8 or voc == 12 then
  local k = items[1]
  if getPlayerSkillLevel(cid, sword) > getPlayerSkillLevel(cid, club) and getPlayerSkillLevel(cid, sword) > getPlayerSkillLevel(cid, axe) then
  doPlayerAddItem(cid, k[1], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[1])..".")
  end
  if getPlayerSkillLevel(cid, club) > getPlayerSkillLevel(cid, sword) and getPlayerSkillLevel(cid, club) > getPlayerSkillLevel(cid, axe) then
  doPlayerAddItem(cid, k[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[2])..".")
  elseif getPlayerSkillLevel(cid, axe) > getPlayerSkillLevel(cid, sword) and getPlayerSkillLevel(cid, axe) > getPlayerSkillLevel(cid, club) then
  doPlayerAddItem(cid, k[3], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[3])..".")
  end
  end
  setPlayerStorageValue(cid, storage, 1)
  end
  if(getPlayerStorageValue(cid, storage) > 0) then
  doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "It is empty.")

  end
return TRUE
end

You can check if your codes are good syntactically on this page: http://ideone.com/
Remember about select "LUA".
 
Upvote 0 Downvote
Oh! Thank you so much! I've made some editing now, to shorten the skill comparisons, will this work too? I mean, in runtime

Code: 
-- Scripted by Ramiro Jesse Grant for Phoenix OTS
local storage = 51222
local voc = getPlayerVocation(cid)
local count = 1
local bag = 2330
local sword = getPlayerSkillLevel(cid, 2)
local axe = getPlayerSkillLevel(cid, 3)
local club = getPlayerSkillLevel(cid, 1)
local items = {
  [1] = {7407, 2445, 7455}, -- Knight Items
  [2] = {8868, 2122}, -- Druid/Sorcerer Items
  [3] = {8858, 2352}  -- Paladin Items
}
function onUse(cid, item, frompos, item2, topos)
  if getPlayerStorageValue(cid, storage) == -1 then
  if voc == 1 or voc == 5 or voc == 9 or voc == 2 or voc == 6 or voc == 10 then
  local m = items[2]
  doPlayerAddItem(cid, bag, 1)
  doAddContainerItem(bag, m[1], count)
  doAddContainerItem(bag, m[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado un "..getItemNameById(bag).." que contiene una "..getItemNameById(m[1]).." y un "..getItemNameById(m[2])..".")
  end
  if voc == 3 or voc == 7 or voc == 11 then
  local p = items[3]
  doPlayerAddItem(cid, bag, 1)
  doAddContainerItem(bag, p[1], count)
  doAddContainerItem(bag, p[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(bag).." que contiene un "..getItemNameById(p[1]).." y una "..getItemNameById(p[2])..".")
  end
  if voc == 4 or voc == 8 or voc == 12 then
  local k = items[1]
  if sword > club and sword > axe then
  doPlayerAddItem(cid, k[1], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[1])..".")
  end
  if club > sword and club > axe then
  doPlayerAddItem(cid, k[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[2])..".")
  elseif axe > sword and axe > club then
  doPlayerAddItem(cid, k[3], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[3])..".")
  end
  end
  setPlayerStorageValue(cid, storage, 1)
  end
  if(getPlayerStorageValue(cid, storage) > 0) then
  doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "It is empty.")
  end
return TRUE
end

Awesome page you shared! It's great! I'm very happy with the script..I didn't test it lol, I hope it works..
 
Upvote 0 Downvote
Here is how I would write this script:
Code: 
-- Scripted by Ramiro Jesse Grant for Phoenix OTS

local storage = 51222
local count = 1
local bag = 2330
local items = {
    [1] = {7407, 2445, 7455}, -- Knight Items
    [2] = {8868, 2122}, -- Druid/Sorcerer Items
    [3] = {8858, 2352}  -- Paladin Items
}

function onUse(cid, item, frompos, item2, topos)
    local voc = getPlayerVocation(cid)
    local sword = getPlayerSkillLevel(cid, 2)
    local axe = getPlayerSkillLevel(cid, 3)
    local club = getPlayerSkillLevel(cid, 1)

    if getPlayerStorageValue(cid, storage) < 0 then
        if isInArray({1, 5, 9, 2, 6, 10}, voc) then
            local m = items[2]
            doPlayerAddItem(cid, bag, 1)
            doAddContainerItem(bag, m[1], count)
            doAddContainerItem(bag, m[2], count)
            doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado un "..getItemNameById(bag).." que contiene una "..getItemNameById(m[1]).." y un "..getItemNameById(m[2])..".")
        end
        if isInArray({3, 7, 11}, voc) then
            local p = items[3]
            doPlayerAddItem(cid, bag, 1)
            doAddContainerItem(bag, p[1], count)
            doAddContainerItem(bag, p[2], count)
            doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(bag).." que contiene un "..getItemNameById(p[1]).." y una "..getItemNameById(p[2])..".")
        end
        if isInArray({4, 8, 12}, voc) then
            local k = items[1]
            if sword > axe and sword > club then
                doPlayerAddItem(cid, k[1], count)
                doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[1])..".")
            elseif club > sword and club > axe then
                doPlayerAddItem(cid, k[2], count)
                doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[2])..".")
            elseif axe > sword and axe > club then
                doPlayerAddItem(cid, k[3], count)
                doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[3])..".")
            else
                doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "Could not determine your main skill.")
                return FALSE
            end
        end
        setPlayerStorageValue(cid, storage, 1)
    else
        doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "It is empty.")
    end
    return TRUE
end

My suggestions?

Using isInArray() function instead of multiple if's makes your code look better.
I think putting this:
Code: 
local voc = getPlayerVocation(cid)
local sword = getPlayerSkillLevel(cid, 2)
local axe = getPlayerSkillLevel(cid, 3)
local club = getPlayerSkillLevel(cid, 1)
out of onUse() function is incorrect as these values have to be updated each time a player uses the item (I may be wrong).
One more thing I'd mention is your:
Code: 
if getPlayerStorageValue(cid, storage) == -1 then
...
end
if(getPlayerStorageValue(cid, storage) > 0) then
...
end

Next time try to use this instead as it does the same:
Code: 
if getPlayerStorageValue(cid, storage) < 0 then
...
else
...
end

That's all for the code itself. Now I want you to imagine a player with vocation 4, 8 or 12 whose axe, sword and club skills are on the same level. In such case your code will only set a storage value without giving him any reward. That's why your statement should look like this:
Code: 
if sword > axe and sword > club then
...
elseif club > sword and club > axe then
...
elseif axe > sword and axe > club then
...
else
...
end
 
Upvote 0 Downvote
Code: 
-- Scripted by Ramiro Jesse Grant for Phoenix OTS
local storage = 51247
local count = 1

local items = {
  [1] = {7407, 2445, 7455}, -- Knight Items
  [2] = {8868, 2122}, -- Druid/Sorcerer Items
  [3] = {8858, 2352}  -- Paladin Items
}

function onUse(cid, item, frompos, item2, topos)

   local voc = getPlayerVocation(cid)
   local sword = getPlayerSkillLevel(cid, 2)
   local axe = getPlayerSkillLevel(cid, 3)
   local club = getPlayerSkillLevel(cid, 1)
 
  if getPlayerStorageValue(cid, storage) < 0 then
  if isInArray({1, 5, 9, 2, 6, 10}, voc) then
  local m = items[2]
  doPlayerAddItem(cid, m[1], count)
  doPlayerAddItem(cid, m[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(m[1]).." y un "..getItemNameById(m[2])..".")
  end
  if isInArray({3, 7, 11}, voc) then
  local p = items[3]
  doPlayerAddItem(cid, p[1], count)
  doPlayerAddItem(cid, p[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado un "..getItemNameById(p[1]).." y una "..getItemNameById(p[2])..".")
  end
  if isInArray({4, 8, 12}, voc) then
  local k = items[1]
  if sword > club and sword > axe then
  doPlayerAddItem(cid, k[1], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[1])..".")
  elseif club > sword and club > axe then
  doPlayerAddItem(cid, k[2], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[2])..".")
  elseif axe > sword and axe > club then
  doPlayerAddItem(cid, k[3], count)
  doPlayerSendTextMessage(cid,22,"Felicidades "..getPlayerName(cid).."! Has encontrado una "..getItemNameById(k[3])..".")
  else
   return doPlayerSendCancel(cid, 'Necesitas una skill mas alta que la otra.')
  end
  end
  else
  setPlayerStorageValue(cid, storage, 1)
  elseif (getPlayerStorageValue(cid, storage) > 0) then
  doPlayerSendTextMessage(cid, MESSAGE_INFO_DESCR, "It is empty.")
  end
return TRUE
end

There it is! It works in my server without problems, so..I don't know if this could be shortened..But I'm happy with this work, thanks to everyone! And thanks Redan and Rer for the help! I didn't know of the isInArray function, it's great!
 
Upvote 0 Downvote

Similar threads

samuel157
  • Solved
Lua [SQLite] -=[TFS]=- 0.4 8.60 How can I fix this script with problem in storage vip
Replies
3
Views
316
Mateus Robeerto
Mateus Robeerto
G
  • Solved
RevScripts Autoloot-creaturescripts (OTX-8.6)
Replies
3
Views
415
Gaber Zen
G
J
  • Question
Autoloot Problem tfs 0.4
Replies
1
Views
293
jareczekjsp
J
J
  • Question
Problem autoloot
Replies
8
Views
585
Obito
Obito
J
  • Question
Castle 24 tfs 1.5 Problem
Replies
0
Views
216
jareczekjsp
J
K
  • Solved
Lua NPC give first weapon for knight (my first script)
Replies
2
Views
275
kibo433
K
M
  • Question
Onkill otx2
Replies
6
Views
528
matheusjp2
M
Sebastian Ching
  • Question
Lua -Otserv 7.4 - 7.72 - error in console
Replies
6
Views
307
Sebastian Ching
Sebastian Ching
manc
  • Question
TFS 0.X TFS 0.X (OTX 2) - You have found a chest. HELP!
Replies
2
Views
284
manc
manc
J
  • Question
Auction system Problem
Replies
0
Views
248
jareczekjsp
J
Back
Top