mapArea iterator

[Colandus]

Hey. This is a revised version of my old mapArea iterator. This simplifies looping through areas.

My old iterator was a bit more confusing for developers who might have wanted to edit the iterator function itself. This time it's using coroutines so the function should be very easily customizable.

The old iterator was using stackpos, this one isn't. I assume it's irrelevant these days? It also fetched the creature on the tile however, which this one doesn't. But I plan to add it. I wanted some input from you guys how I should approach this. Just return top thing as second value or should we have an option to get e.g. TOP_CREATURE or TOP_ITEM or an option to allow it to loop through each item on every tile (without the necessity of a second loop)? Or a callback? Possibility to reverse loop?

I feel like this function never was much appreciated back when I made this, which confuses me. I fail to understand why anyone would like to create 3 loops in place of a single one, which also might save you a few more lines checking for a player etc...

Enough talk, here's the code, place this wherever you place your functions normally:

do
    local function mapAreaGen(fromPos, toPos)
        for x = fromPos.x, toPos.x do
            for y = fromPos.y, toPos.y do
                for z = fromPos.z, toPos.z do
                    coroutine.yield(Position(x, y, z))
                end
            end
        end
    end

    function mapArea(fromPos, toPos)
       local co = coroutine.create(function () mapAreaGen(fromPos, toPos) end)
       return function()
           local _, pos = coroutine.resume(co)
           return pos
       end
    end
end

Simple example usage:

local fromPos = Position(...)
local toPos = Position(...)
for position in mapArea(fromPos, toPos) do
   -- position:sendMagicEffect(CONST_ME_MAGIC_BLUE)
end

COMPARED to current way of looping:

local fromPos = Position(...)
local toPos = Position(...)
for x = fromPos.x, toPos.x do
   for y = fromPos.y, toPos.y do
      for z = fromPos.z, toPos.z do
         local position = Position(x, y, z)
         -- position:sendMagicEffect(CONST_ME_MAGIC_BLUE)
      end
   end
end

If you are intending to use this function, don't forget to come back and look for updates in short. In the meantime, I would like some input on how I should be adding the thing, creature etc...

 

[Infernum]

using coroutines for no reason isnt exactly more efficient
yes it works but it's not needed
if you compare the times of your function vs the old loop in a 1000x1000x1 area your function is almost twice as slow

if you still wanted to create a function for this instead of regular looping you could pass a function through mapArea like:
mapArea(fromPos, toPos, func, ...)
that way the user can define what exactly they want to do with their own function

min, max = math.min, math.max

function mapArea(fromPos, toPos, func, ...)
    for z = min(fromPos.z, toPos.z), max(fromPos.z, toPos.z) do
        for x = min(fromPos.x, toPos.x), max(fromPos.x, toPos.x) do
            for y = min(fromPos.y, toPos.y), max(fromPos.y, toPos.y) do
                local position = Position(x, y, z)
                func(position, ...)
            end
        end
    end
    return true
end

local function cleanPosItemsById(position, itemid)
    local tile = Tile(position)
    if not tile then
        return false
    end
    local tileItem
    for index = 0, tile:getItemCount() do
        tileItem = tile:getThing(index)
        if (tileItem:getId() == itemid) then
            tileItem:remove()
        end
    end
    return true
end

mapArea(Position(994, 997, 7), Position(1007, 1004, 7), cleanPosItemsById, 2160)

 

[496815]

@Colandus I actually did appreciate your old function and have used it a lot, I did modify it a bit though.

Regarding monsters / items: I'd just make new functions for those to be honest, seems messy enough as it is.

 

[Colandus]

@Xeraphus Twice as slow isn't much, talking microseconds. Coroutines for no reason? Coroutines aren't slow though and there is a good reason; code is much cleaner, if you had seen the old mapArea iterator. But if you want to compare only with the regular for loops, you gain cleaner code and easier usability in exchange for an insignificant increase of workload.

While your example isn't bad, I'm not a fan of using callbacks too much. Just a personal preference, hence why I tend to make these types of iterators. I want to use the loop without the need of nesting 3 loops each time. Saving lines I guess, not just for the result but for the time it takes typing them. By also returning a creature or item or similar I would save more lines than just writing less loops. Also this is directed to scripters of all levels, and most of them get confused by callbacks, not to mention a bunch of nested loops. Frankly I rarely need to use this iterator myself. But I suppose that's because I rarely need to scan areas as I use other better methods for whatever it is I'm going to do.

Also that upvalue of tileItem is redundant and also slower than adding it directly inside the loop. As well recalculating the min and max on each iteration is slow.

@496815 Yeah, I just feel it's more convenient if you'd like to look for e.g. players only, you won't need to add an extra variable (local player = Tile(pos):getTopVisibleCreature(), or at worst loop getCreatures) and an additional if-statement. For now I'm considering using flags or perhaps as bot scripts tend to have it a string with e.g. "pmn" to select player, monster, npc. Not sure yet.

My main usage of such functions is actually using live in-game codes. You want them as short as possible. I tend to write lots of in-game codes each day.

 

[Infernum]

@Xeraphus Twice as slow isn't much, talking microseconds. Coroutines for no reason? Coroutines aren't slow though and there is a good reason; code is much cleaner, if you had seen the old mapArea iterator. But if you want to compare only with the regular for loops, you gain cleaner code and easier usability in exchange for an insignificant increase of workload.

While your example isn't bad, I'm not a fan of using callbacks too much. Just a personal preference, hence why I tend to make these types of iterators. I want to use the loop without the need of nesting 3 loops each time. Saving lines I guess, not just for the result but for the time it takes typing them. By also returning a creature or item or similar I would save more lines than just writing less loops. Also this is directed to scripters of all levels, and most of them get confused by callbacks, not to mention a bunch of nested loops. Frankly I rarely need to use this iterator myself. But I suppose that's because I rarely need to scan areas as I use other better methods for whatever it is I'm going to do.

Also that upvalue of tileItem is redundant and also slower than adding it directly inside the loop. As well recalculating the min and max on each iteration is slow.

My main usage of such functions is actually using live in-game codes. You want them as short as possible. I tend to write lots of in-game codes each day.

i know twice as slow isn't much when we're talking about a small area, but you're releasing code for the community.
you should make it efficient and fool-proof because you don't know what people are going to do with it + make it customizable to the user's preference rather than your preference

callbacks aren't exactly a very hard thing to understand, a good example of using a callback is in addEvent, which evan has a tutorial for in the tutorials section.

 

[Colandus]

i know twice as slow isn't much when we're talking about a small area, but you're releasing code for the community.
you should make it efficient and fool-proof because you don't know what people are going to do with it + make it customizable to the user's preference rather than your preference

callbacks aren't exactly a very hard thing to understand, a good example of using a callback is in addEvent, which evan has a tutorial for in the tutorials section.

Sometimes readability and convenience is to be considered before performance if the difference isn't significant. Obviously I tend to make it customizable, perhaps you didn't read my post and just skipped to the code. And as you said, it's about giving the users options right? Well, anyone can make a function that takes a callback. At least I'd imagine anyone who can use a callback can write such a function themselves. And if they cannot, well you have provided them with one right? So that's good. Now they have options. But those who prefer to use loops, they may use this iterator. If loops weren't necessary, ipairs would be a callback and all other iterators and loops in general. Loops are sometimes convenient.

 

[Colandus]

i know twice as slow isn't much when we're talking about a small area, but you're releasing code for the community.
you should make it efficient and fool-proof because you don't know what people are going to do with it + make it customizable to the user's preference rather than your preference

callbacks aren't exactly a very hard thing to understand, a good example of using a callback is in addEvent, which evan has a tutorial for in the tutorials section.

I benchmarked and yours is ~35% slower even when removing the math.min/max calls. Coroutines are faster in Lua, functions are slow.


Compared to my old mapArea without coroutines (but more complex code), the new one using coroutines is only about 3,5% slower. Here is what it looks like:

function mapArea(fromPos, toPos)
    local x, y, z = fromPos.x, fromPos.y-1, fromPos.z
    return function()
        if (y < toPos.y) then
            y = y+1
        elseif (x < toPos.x) then
            y = fromPos.y
            x = x+1
        elseif (x == toPos.x and y == toPos.y and z < toPos.z) then
            x = fromPos.x
            y = fromPos.y
            z = z+1
        else
            x = x + 1
        end
        if (x <= toPos.x and y <= toPos.y or z < toPos.z) then
            return Position(x, y, z)
        end
    end
end

So if anyone would care about such insignificant performance loss for the cost of less readable code, then they'd be free to use this version. Obviously, if someone preferred to use callbacks they may do so too. Although, it should not be about performance but rather coding style and preference.