Modding Help Help with world.liquidAt(position)

Hi everyone, currently this bit of code just lags my game sending for quite the loop..

Code:

function teleport( position)
    blinkFootOffset = 5

    if world.liquidAt({position[1], position[2] + blinkFootOffset}) then
        tech.burstParticleEmitter("lowenergy")
        else
        tech.burstParticleEmitter("cooldown")
    end

end

So I've tested all parts of the function the position works and exists, and no matter what I do with world.liquidAt() it just causes lag and what seems like an infinite loop, never entering the particleEmitter test.

 

Give us the whole code. Simply giving us this one function is useless to us. What calls this function, where, how often,etc.

A simple If statement will not enter an infinite loop, nor should it cause "Lag". Your problem is elsewhere.

 

I think he means that he suspects it's getting executed constantly, causing latency. But yes, we need the entire .lua. The tech that calls it would be nice, too.

 

Hence my statement saying that the problem is elsewhere.

 

I can assure you that the world.liquidAt(position) is the problem... the entire tech is 1000+ lines of code and I didn't want to post all that. For note

Code:

--- Begin Teleport ---

function teleport(safeDistance, position)
    blinkFootOffset = 5

test= world.liquidAt({position[1], position[2] + blinkFootOffset})


tech.setPosition({position[1],position[2]+20+safeDistance})
data.teleportTimer=1
data.targetPosition=tech.position()
  
end

Will cause the massive amount of lag.

Code:

--- Begin Teleport ---

function teleport(safeDistance, position)
    blinkFootOffset = 5

-- test= world.liquidAt({position[1], position[2] + blinkFootOffset})


tech.setPosition({position[1],position[2]+20+safeDistance})
data.teleportTimer=1
data.targetPosition=tech.position()
  
end

This will not cause any lag and work as intended.

 

Do me a favor. Put a logger inside your teleport function and tell me how many times it runs.

For instance.

Code:

function teleport(safeDistance, position)
    blinkFootOffset = 5
world.logInfo("I assure you liquidAt is not the problem!")
test= world.liquidAt({position[1], position[2] + blinkFootOffset})


tech.setPosition({position[1],position[2]+20+safeDistance})
data.teleportTimer=1
data.targetPosition=tech.position()
 
end

But thats fine... If you dont want to listen to me or take my advice, then have fun figuring it out yourself.

 

Trying that out now, will give you results shortly.

Code:

Info: I assure you liquidAt is not the problem!
Error: Exception while invoking lua method 'update'. LuaException: [string "/magic/magitech/magiTech.lua"]:521: attempt to call field 'liquidAt' (a nil value)

Is what came up. Many times. First time I've tried the log file, didn't even know that I could run these logs, thank you for that.

Though I am unsure why it's saying a nil value, tech.position() is a valid coordinate. The reason it is lagging is it is breaking out of the function once it hits world.liquidAt(tech.position()), because that is throwing an error . If I move the data.teleportTimer=1 to the top it will only iterate every second, because it is supposed to iterate until a certain criteria happens. If the function is breaking then it will loop.

I suppose my main question at this point is what is it saying "(a nil value)".

 

So, not only are you looping/calling the function too often some how (likely though another function?), but I also COMPLETELY forgot that world.liquidAt() is not a valid/working function in this version of starbound for techs. Used to work prior too! Even used in vanilla assets like the Blink tech, which makes no sense.

Edit: However upon closer look of the blink tech... the world.liquidAt() is NEVER called since all the possible cases the game passes in false for whether or not to check for water, thus skipping over the world.liquidAt() call... Explains why no one reported errors then I guess.

Edit 2:
No mate. An error will only throw once each time it is called. If you are seeing hundreds of these lines being outputted then you are calling the function hundreds of times which is what I had said from the very beginning. From what it sounds, your logic is incorrect for the timing and the teleport function is being called WAY too often. However, since you still refuse to show me the code you have, I cannot help you diagnose the issue.

 

So is it confirmed that world.liquidAt() simply doesn't work anymore? I had enabled it on blink and yes caused the same error, which is why I was wondering at the start if world.liquidAt(position) was just broken. Which according to you is the issue.

The rest of the code works well, as in the iteration happens the amount of times it should, as long as it doesn't break out of the function, else as you stated the function will continually be called. I had simply hoped to not teleport my players into lava any more.

As for the code, as I stated it's 1000+ lines and the only pertinent part was that, however if you wish here it is for your viewing.

http://pastebin.com/e5703vcV

Teleport function is at 515, called at 680.

I would appreciate help in some way of detecting if where my players next position is going to be lava.

 

Dude you SERIOUSLY need some lessons on programming.
90% of those enormous if statements could be made into tables.

Example:

Code:

--YOUR VERSION
    function displaySpell(test)
    local spellPrep = nil

    -- Displaying if an attack spell got off
            if test == "2" then
                    spellPrep = "magicarrowText"
            elseif test == "12"  then
                    spellPrep = "sparkText"
            elseif test == "42"  then
                    spellPrep = "fireblastText"
            elseif test == "142"  then
                    spellPrep = "rapidfireText"
            elseif test == "32" then
                    spellPrep = "frostwaveText"
            elseif test == "422"  then
                    spellPrep = "fireballText"
            elseif test == "3142" then
                    spellPrep = "blackholeText"
            elseif test == "44332"  then
                    spellPrep = "chainlightningText"
            elseif test == "31423142"  then
                    spellPrep = "thunderstormText"
            elseif test == "33441212" then
                    spellPrep = "meteorswarmText"
            elseif test == "11" then
                    spellPrep = "meteorswarmText"
             
    -- Displaying if a heal/buff spell got off
            elseif test == "1"  then
                    spellPrep = "regenerationText"   
            elseif test == "4"  then
                    spellPrep = "warmthText"         
            elseif test == "14"  then
                    spellPrep = "breatheText"         
            elseif test == "34"  then
                    spellPrep = "jumpText"
            elseif test == "24" then
                    spellPrep = "hasteText"
            elseif test == "2314" then
                    spellPrep = "vigorText"
            elseif test == "241" then
                    spellPrep = "hawkeyeText"
            elseif test == "344"  then
                    spellPrep = "greaterjumpText"
            elseif test == "414"  then
                    spellPrep = "enviroprotectText"
            elseif test == "2134"  then
                    spellPrep = "heroismText"
            elseif test == "2244"  then
                    spellPrep = "greaterhasteText"
            elseif test == "3421"  then
                    spellPrep = "healText"
     
    -- Displaying if movement spells are activated
            elseif test == "33"  then
                    spellPrep = "galeText"
            elseif test == "21"  then
                    spellPrep = "blinkText"
            elseif test == "44331"  then
                    spellPrep = "teleportText"
            elseif test == "31"  then
                    spellPrep = "flyText"

    -- Displaying if a summon spell got off (Activate would be a much better way of saying it)
            elseif test == "3"  then
                    spellPrep = "flareText"
            elseif test == "43" then
                    spellPrep = "createwaterText"
            elseif test == "143"  then
                    spellPrep = "guardText"
            elseif test == "413"  then
                    spellPrep = "arcaneeyeText"
            elseif test == "243"  then
                    spellPrep = "eternalglowText"
            elseif test == "14243"  then
                    spellPrep = "legionText"
            elseif test == "31423" then
                    spellPrep = "summonstarText"
            elseif test == "34122143"  then
                    spellPrep = "summondragonText"
            end

            if spellPrep ~= nil then
                    tech.burstParticleEmitter(spellPrep)
            end

    end

My version is only5 lines:

Code:

    -- MY VERSION
     validSpells = {spell1 = "somespell" , spell2343243 = "lol another spell"  }
    spellPrep = validSpells["spell" .. test]
    if spellPrep ~= nil then
            tech.burstParticleEmitter(spellPrep)
    end

 

An example would be helpful, if you wouldn't mind showing me what you mean.

 

Edited into it. Also just fixed my assignment error.

Also. Jesus christ man your Update() function is over 400 lines of if statements, world calls, and other expensive ops. You realize update() runs every frame right? No wonder you game is lagging!

 

Honest discussion, since you may or may not be aggressive, can't tell because of lack of tone in text. However,

Code:

 -- MY VERSION
    validSpells = {"1" = "somespell" , "2343243" = "lol another spell"  }
    spellPrep = validSpells[test]
    if spellPrep ~= nil then
            tech.burstParticleEmitter(spellPrep)
    end

Wouldn't

Code:

    spellPrep = validSpells[test]

Mean that when I pass "test" through into the function it be a number from 1 to infinity. Thus having to number out the spells in a specific order. Or can I actually pass "2343243" (for example) and get "lol another spell". Honest question as I have never programmed in a high level language before.

 

I edited it prior to your finished response... You cant say "1" = "somespell", instead you have to say spell1 = "somespell" due to the way table assignment works, but yes thats the gist of it. You pass in the test parameter of 2343243, and you will get spellPrep set to "lol another spell". They do not have to be in a specific order either.

Test is the variable you chose as the parameter for the function... though I have no clue why... or what it even means.

 

I feel like that last line is a jab at me using a poor naming convention

Thank you for that programming tip, I can see you looking at that why you would assume I needed programming lessons, though I will fully admit that in high level code I am a complete novice. Things like this seem the realm of database, rather then something to be beautifully implemented into a language. I will happily make further use of tables to condense my code, and can clearly see where I can use them to improve on data structures. I had assumed they just worked like arrays, my mistake.

 

Umm.. the game doesn't lag, only when I had a function call a broken class, which caused the first function to no longer finish creating a loop it caused lag. But those 400 lines of if statements and world calls are for the most part 16 if statements per frame, which is roughly 40 instructions per cycle.

Unless the update attempts to run every if statement even if it doesn't have to (nested), then it would be laggy for sure.

For example for clairity,

Code:

update()
if x ==1 then

   if y ==2 then
      do 400 calls
    end
end

end

So given the code above it will only lag if y is always equal to 2, else it ignores everything inside that. Correct? Assuming that the 400 calls and such won't lag if run for a single cycle/frame.

 

While your assessment is correct, it does not MATTER if you are not causing the game to lag. Poorly written code is bad code. Poorly written code is inefficient.

For example
I went from THIS

Code:

-- Function to check if current spell is valid
function validSpell(test)
if data.spellbook > 0 then
        if test == "1" or test == "2" or test == "4" or test == "3" or test == "11" then
                data.spellCurrent = test
                data.spellTier=1
                displaySpell(test)
        end
        if data.spellbook > 1 then
                if test == "33" or test == "14"  or test == "12" then
                        data.spellCurrent = test
                        data.spellTier=2
                        displaySpell(test)
                end
                if data.spellbook > 2 then
                        if      test == "34" or test == "43" or test == "42" or test == "44331" then
                                data.spellCurrent = test
                                data.spellTier=3
                                displaySpell(test)
                        end
                        if data.spellbook > 3 then
                                if test == "24" or test == "142" or test == "143"  then
                                        data.spellCurrent = test
                                        data.spellTier=4
                                        displaySpell(test)
                                end
                                if data.spellbook > 4 then
                                        if test == "2314" or  test == "413" or test == "32" or test == "21" then
                                                data.spellCurrent = test
                                                data.spellTier=5
                                                displaySpell(test)
                                        end
                                        if data.spellbook > 5 then
                                                if test == "241" or  test == "422" or test == "243"  then
                                                        data.spellCurrent = test
                                                        data.spellTier=6
                                                        displaySpell(test)
                                                end
                                                if data.spellbook > 6 then
                                                        if test == "344" or test == "3142" or test == "414" then
                                                                data.spellCurrent = test
                                                                data.spellTier=7
                                                                displaySpell(test)
                                                        end
                                                        if data.spellbook > 7 then
                                                                if test == "44332" or test == "3421" or test == "14243" then
                                                                        data.spellCurrent = test
                                                                        data.spellTier=8
                                                                        displaySpell(test)
                                                                end
                                                                if data.spellbook > 8 then
                                                                        if test == "2244" or test == "31423142" or test == "31423" then
                                                                                data.spellCurrent = test
                                                                                data.spellTier=9
                                                                                displaySpell(test)
                                                                        end
                                                                        if data.spellbook > 9 then
                                                                                if test == "33441212" or test == "31" or test == "2134" or test == "34122143" then
                                                                                        data.spellCurrent = test
                                                                                        data.spellTier=10
                                                                                        displaySpell(test)
                                                                                end
                                                                        end
                                                                end
                                                        end
                                                end
                                        end
                                end
                        end
                end
        end
end
end

to this:

Code:

local spellLevels = {
    {1,2,3,4,11}, -- Level 1
    {33,14,12}, -- level 2
    {34,43,42,44331}, -- level 3
    {24,142,143}, -- level 4
    {2314,413,32,21}, -- level 5
    {241,422,243}, -- etc
    {344,3142,414},
    {44332,3421,14243},
    {2244,31423142,31423},
    {33441212,31,2134,34122143}}

function validSpell(spell)
     for k, v in pairs(spellLevels[data.spellbook]) do
         if spell == tostring(v) then
             data.spellCurrent = spell
             data.spellTier= data.spellbook + 1
             displaySpell(spell)
             return
        end
    end
end

I just reduced your 79 lines of redundant if statements (which ALL ran by the way since 9 passes as true from 0 - 8.) Not to mention you were not returning should you get a correct match in your Or statements. So if spellbook was 9, but you matched the VERY first line at test == 1, guess what... every if statement still runs and does nothing.

I may be coming off as rude, or an asshole here... but pushing out code like this is just ASKING to break. Not to mention you have almost 0 meaningful variable names. Like what the hell is "test" anyway? You have no comments in your code, so should you step away from it, you will have to relearn EVERYTHING about what you did.

Learn a little bit more about programming, and fix this. You will benefit both your work, and yourself.

 

You do come across as rude, hyperbolic and stroking your own ego, but I get the point of what you are saying and I very much appreciate the dialogue.

100% right about the returns, that was purely bad coding. As I stated about tables, not a thing I knew about will learn and clearly saw their use as soon as you explained them to me.

Also yes, I know test was a terrible name, forgive me?

A question, though I love your completed code, wouldn't

Code:

function validSpell(spell)
    for k, v in pairs(spellLevels[data.spellbook]) do
        if spell == tostring(v) then
            data.spellCurrent = spell
            data.spellTier= data.spellbook + 1
            displaySpell(spell)
            return
        end
    end
end

Only check it if it was a valid spell of that current book. As example of spellbook 5 you have {2314,413,32,21}. In your code above, it would only check against those? So something like

Code:

function validSpell(spell)
  for i,data.spellbook,1 do
    for k, v in pairs(spellLevels[i]) do
        if spell == tostring(v) then
            data.spellCurrent = spell
            data.spellTier= i
            displaySpell(spell)
            return
        end
    end
  end
end

Be required to make sure it looked through all previous tables? If so, depending if tables are as efficient or greater efficiency as the comparative statements, this would be about the same on the processor , had I not forgot the returns. Though clearly eat up a lot less space on the drive as the code is far more condensed. 89 lines to 26 with your help.

 

I'm not trying to be an ass about it, but it needs to be said. Just because it works, doesn't make it good.

As for the above, You are completely correct. However, this reiterates a previous point about documentation. If you had comments in the code that explains which parts do what, I should be able to understand what you are trying to do and how. If I as a programmer have trouble understanding your code without having to go through it line by line, then you have failed. If you stepped away from this project for over a week, then came back to it, you would be seeing what I see now. My face plastered on the screen with a big "WTF" written across my face. The only way for me to understand what is going on is to go through it line by line.

The code you posted as a "fix" to mine still makes no sense to me. You are still going to exit the function as soon as you get the first match. Your code and mine are completely equivalent aside from the fact that you check starting from 0, and not just the current level of spells. Is this intended? If so, then the code will be just as efficient as your if statements, but admit me this: Its a HELL of alot easier to read and maintain isn't it? Want to add a new spell? Simply insert it into the table above. Thats it. Your method would require finding the appropriate if statement and making SURE you are on the right level, and then adding it to the clusterF!@# of if statements allready.

Edit:
Take this code from my Colonies mod:

Code:

-- Requests a villager to become a merchant. Returns Nil
function requestMerchant(id)
    if not self.merchants then
        self.merchants = {}
    end

    -- Loop through NPCS, find one that does not have an occupation and return its ID.
    -- Also set its occupation.
    local merchant = self.merchants[id]
    world.logInfo("COLONIES: MERCHANT ARG INFO: %s", merchant)
    for k,v in pairs(self.npcIds ) do
        local occupation = world.callScriptedEntity(k, "getOccupation")
        if occupation == "None" then
            -- Assign NPC's occupation to be a merchant
            world.callScriptedEntity(k, "setMerchant", merchant)
            world.logInfo("COLONIES: Called setMerchant on merchant " .. tostring(k))
            return
        end
    end

end

Without even looking at the code, the first comment tells me what it does, and what it returns. Then the Other 2 statements tell me what I am doing without even looking at the code. So I abandon the project for a week or even a year, I can come right back to it and understand everything with minimal effort, also keeping my train of thought in synch with what I was thinking when I originally wrote it.

Edit 2: Just saw your comment about you not being able to read my tone. Must have skimmed over it.
My tone is of casual conversation. I am not trying to be a dick, I am not trying to sound condescending, I am merely here trying to help you learn, and teach you some better practices. That is all.

 

I am fully aware you aren't trying to be rude or hyperbolic, this just seems to be who you are right now, I've read your work and posts before so I get it. Don't worry about how you come across, people will think what they want, but the lessons in this thread are valuable, which is the point.

To the "FIX", yes it' will start at... gosh dang it I forgot i=1... anyway... it will start at 1, and go through to the current levels of spells accessible to the player, returning when it finds a match, which is the point of the validspell check. The variable data.spellbook (see there are descriptive variables), is in the current held spell book. Based on the spellbook is what spells are available to the player. Hence the extra for statement.

The cluster of if statements is an assembly comparative structure before case's existed. I forgot the "goto 10" in the or statements.

Yes, lord yes I admit it is 100% easier to maintain. I appreciate learning about tables from this thread and being able to sift through something in my code like a database, without having to have to write a database. I admit this, and for clarity am exceptionally thankful for. Things I have learned from this thread.

1. It was world.liquidAt(position) fault, since it was deprecated.
2. Tables, can be referenced in pairs and by a stored value. Wooooo!
3. Any poor naming convention will still lose you major points with programmers.

So to further discussion any thoughts on how to detect if there is lava on a particular x/y?