Spreading a table out over several ticks

Place to get help with not working mods / modding interface.
FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Spreading a table out over several ticks

Post by FuryoftheStars »

I'm working on making some changes to another mod, and they have a section of code that spreads updating their entities (stored in a table) over several ticks.

My questions are:
  1. Does the below code snippet actually work (correctly)?
  2. Is there a better way (or is it "good enough")?
(within control.lua)

Code: Select all

local update_interval = 61
script.on_event(defines.events.on_tick, function(event)
    local offset = event.tick % update_interval
    -- code were "table" is defined; basically a table of entities this mod added that all players have built, so can range from empty to positive x 10s, 100s, 1000s, etc of elements.
    for i=#table - offset, 1, -1 * update_interval do
        -- code working with table
    end
    -- other code
end)
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

User avatar
DaveMcW
Smart Inserter
Smart Inserter
Posts: 3699
Joined: Tue May 13, 2014 11:06 am
Contact:

Re: Spreading a table out over several ticks

Post by DaveMcW »

Here is how I do it in burner fuel bonus. global.burners is the table, global.burner_index is the next index to read. Note that I increment the index before using it, this allows me to delete a table entry in the update function without crashing the loop.

Code: Select all

script.on_event(defines.events.on_tick, function(event)
  local my_settings = settings.global
  -- Update N entities per tick
  for i = 1, my_settings["burner-fuel-bonus-refresh-rate"].value do
    local index = global.burner_index
    global.burner_index = next(global.burners, global.burner_index)
    update_burner(index)
  end
end)
Last edited by DaveMcW on Mon Apr 25, 2022 2:17 pm, edited 1 time in total.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

DaveMcW wrote:
Mon Apr 25, 2022 2:05 pm
Here is how I do it in burner fuel bonus. global.burners is the table, global.burner_index is the index.

Code: Select all

script.on_event(defines.events.on_tick, function(event)
  local my_settings = settings.global
  -- Update N entities per tick
  for i = 1, my_settings["burner-fuel-bonus-refresh-rate"].value do
    local index = global.burner_index
    global.burner_index = next(global.burners, global.burner_index)
    update_burner(index)
  end
end)
Hmm, ok, thanks. Took me a bit to understand it as I've never used next and have limited understanding of its function, but I think I understand better now.

So, if I'm understanding it correctly, rather than spreading the elements of the table all out over x ticks, you're actually only processing x elements of the table per tick?
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

User avatar
Silari
Filter Inserter
Filter Inserter
Posts: 488
Joined: Sat Jan 27, 2018 10:04 pm
Contact:

Re: Spreading a table out over several ticks

Post by Silari »

The code in the OP is basically a simpler version of code I wrote to do a similar thing in Teleportation Redux.

Code: Select all

  if settings.global["Teleportation-Dynamic-Loop"].value then
    --dynamic speed, attempt every provider over a second.
    local last_index = global.Telelogistics.index_of_last_processed_provider or 0
    local target_index = last_index + math.ceil(#global.Telelogistics.teleproviders/60)
    local current_index = last_index + 1
    while current_index <= target_index do
        --Attempt to process the Provider at the current_index, if one exists.
        --Note it's possible for current_index to be greater than the number of providers, if the count changed within
        --the last second (if some were removed, for example)
        if global.Telelogistics.teleproviders[current_index] then
            Telelogistics_ProcessProvider(current_index)
        end
        current_index = current_index + 1
    end
    --Set our last processed provider to the last target, or 0 
    if target_index > #global.Telelogistics.teleproviders then
        target_index = 0
    end
    global.Telelogistics.index_of_last_processed_provider = target_index
  end
I think code in the OP can end up skipping items if things were added/removed during the last tick, while mine is fine with any additions, or removals past the current index - if it's before the current index it'd skip an item this pass. Wasn't worth trying to code around catching that, and may not be worth the extra complexity for the code in the OP depending on how important what it's doing is.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

Silari wrote:
Tue Apr 26, 2022 2:46 am
The code in the OP is basically a simpler version of code I wrote to do a similar thing in Teleportation Redux.



I think code in the OP can end up skipping items if things were added/removed during the last tick, while mine is fine with any additions, or removals past the current index - if it's before the current index it'd skip an item this pass. Wasn't worth trying to code around catching that, and may not be worth the extra complexity for the code in the OP depending on how important what it's doing is.
Thanks for your input. It's good to know that it at least is working for the most part.

Yeah, the OP code isn't concerned with catching something as it's being added in. I've actually already changed it to extend the number of ticks it processes on from 61 to 300 as it's not that important to even catch something within 1 secs time.

To some degree I even wonder if it's necessary to check every tick, though then the delays might start becoming noticeable? The entities the player places with it (some of which can actually be automated, too) and it needs to monitor actually take several minutes before they reach the stage where the code needs to then intervene and do something, though there can potentially be many of them and not all set up at the same time. At the moment I don't know if there's a better way I can replace it with.

For reference, this all is from one of the tree sapling mods... revived and revisited patch or something? General principal is you can create tree saplings to plant by hand and then harvest for wood once they mature, or automate the whole thing in planters. The mod keeps track of each sapling planted (and their associated planter if there is one for the automation part) and after a randomly selected number of ticks (in the minutes range), replaces the sapling with a regular tree (or if they're in a planter (which is just an assembler with a recipe), it finishes out the recipe for the return of wood).

There were a lot of mistakes in the code (most of which I think came from the original version of the mod back when some of its operations probably made sense but don't now) and even some duplication of effort (looping backwards through a copy of the master list, to pull select items out into another list that it then loops through again (backwards) and performs the specific operations that it applies to the master list) that I've cleaned up. At this point I'm just eyeing this section of code on it's functionality and necessity, and have some print statements in a couple other areas to try and figure out under what conditions they even trigger (they trigger, they do, but so far I've failed to figure out why).
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

User avatar
Silari
Filter Inserter
Filter Inserter
Posts: 488
Joined: Sat Jan 27, 2018 10:04 pm
Contact:

Re: Spreading a table out over several ticks

Post by Silari »

If that's what it's doing it might be best to not bother looping through the list at all. Decide when the sapling is planted how many ticks it'll take to grow, then throw it into a table indexed by tick, with a list of all saplings that are going to grow that tick. All you need to do in a tick is see what saplings grow that tick, if any, then process them all and delete that entry in the table. Won't matter what's been added/removed in a given tick since it doesn't loop.

Depending on how the planter is setup that might still need the looping list, but at the very least normal saplings can save some time.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

Hmm, that might work for the saplings. I'll take a look at if it's possible to recode for it to work that way considering the inter dependency on the planters, though I still gotta figure out why those random couple of sections that rarely trigger are actually triggering.

As for the planters, no, I suspect it will still need to loop (unless there are event handlers for an assembler starting and finishing). When a planter starts, it puts a sapling at the planters location, so you get both the visualization of a growing sapling as well as the pollution absorption from it. Once the planter finishes, it removes it. But still, this would cut the number of entities it has to monitor in half.
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

Thanks both of you for your inputs. At this time I've actually taken both ideas.

For the saplings, I'm using the approach where they are stored in the global.saplings table based on the tick they are meant to "mature" at. Every tick it checks this table to see if there are any saplings set to mature this tick and processes any it finds.

For the planters, I'm storing them in the global.planters table based on their entity.unit_number (for easier lookup when a sapling in a planter matures) and then iterating over them with the next method (seems they are now non-contiguous).

However, I seem to have been able to trigger an "invalid key to next" error in this loop (research shows this is from adding to the table while iterating over it with next). Only once, despite many attempts, but once is enough to know it can happen, so now I'm trying to think of ways to prevent this from possibly happening again.

First thought is to put in a new local var with a default value of false and make it available to the whole control script. When the add function is called, it sets this to true. Inside the planters' next loop, it checks for this and breaks if true, setting it back to false.

Second thought (which I thought of as I was typing this post) is to separate the saplings. Only saplings without planters go into the global.saplings table. Saplings with a planter actually get stored with the respective planter in the global.planters table. Then, I can either process them the same way as regular saplings, but when their planter gets processed, or even use them to tweak the planter's crafting progress (seems recipe lengths are not dynamic) and let the planter finish it out on it's own. I could then revert the way planters are stored and their loop to what it was before.

First thought may be easier to implement, but as I've been thinking about it more and more while typing this post, I'm liking the second method more and more.

Hmmm....
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

Ok, idea 2 may not work. If anything happens to the sapling (it gets mined separate from its planter or somehow destroyed), I don't want to have to loop through the whole planters table to figure out which one it belonged to.

That said, instead of a variable as in idea 1, I think just making a copy of the table to a separate var and then looping over the copy should suffice.
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

robot256
Filter Inserter
Filter Inserter
Posts: 594
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Spreading a table out over several ticks

Post by robot256 »

If I need to traverse a table and modify it based on what I find, I make a local variable to keep a list of changes to make. After going through the list as-is and adding to the change list, I process all the changes at once (in reverse order, if list indexes need to be kept contiguous).

Which table are you modifying while iterating over it?

Pi-C
Smart Inserter
Smart Inserter
Posts: 1639
Joined: Sun Oct 14, 2018 8:13 am
Contact:

Re: Spreading a table out over several ticks

Post by Pi-C »

FuryoftheStars wrote:
Tue May 03, 2022 9:02 pm
Ok, idea 2 may not work. If anything happens to the sapling (it gets mined separate from its planter or somehow destroyed), I don't want to have to loop through the whole planters table to figure out which one it belonged to.
Make 2 tables, one indexed by planter unit_number, with a lookup list of its plants' IDs, the other indexed by plants' unit_number, containing a reference to the planted entity:

Code: Select all

global,planters = {
     [1] = {
         [21] = true,
         [22]  = true,
         [24] = true,
     },
    [2] = {
        [25] = true,
    },
}
global.plants = {
    [21] = { entity = LuaEntity, planter = 1},
    [22] = { entity = LuaEntity, planter = 1},
    [24] = { entity = LuaEntity, planter = 1},
    [25] = { entity = LuaEntity, planter = 2},
}

if planter_is_ready then
	for plant, p in pairs(global.planters[unit_number] do
	    plant = global.plants[plant].entity
	    …
	end
end

if plant_is_dead then
    planter = global.plants[plant.unit_number].planter
    global.planters[planter][plant.unit_number] = nil
end
A good mod deserves a good changelog. Here's a tutorial (WIP) about Factorio's way too strict changelog syntax!

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

robot256 wrote:
Wed May 04, 2022 3:20 am
If I need to traverse a table and modify it based on what I find, I make a local variable to keep a list of changes to make. After going through the list as-is and adding to the change list, I process all the changes at once (in reverse order, if list indexes need to be kept contiguous).

Which table are you modifying while iterating over it?
Both. However, based on the way I'm storing them and iterating over them, order and contiguous is not important, so I can safely remove items as I go. The error I ran into was because, somehow, I managed to trigger the on_built_entity event at the same time the on_tick event was running (adding to the table while iterating with next is apparently a no-no). So I've solved this by making a copy of the table, then iterating over the copy while making the needed changes to the original. Again, as contiguous is not important, I don't re-index it (and even if the add function fires, its not going to affect the other indexes), so this should hold fine.
Pi-C wrote:
Wed May 04, 2022 3:32 am
FuryoftheStars wrote:
Tue May 03, 2022 9:02 pm
Ok, idea 2 may not work. If anything happens to the sapling (it gets mined separate from its planter or somehow destroyed), I don't want to have to loop through the whole planters table to figure out which one it belonged to.
Make 2 tables, one indexed by planter unit_number, with a lookup list of its plants' IDs, the other indexed by plants' unit_number, containing a reference to the planted entity:
Yeah, I'm already indexing planters by unit_number and storing this with their respective saplings. Saplings, on the other-hand, to avoid looping through the whole table, are stored in subtables within the main table indexed by the game.tick they are set to mature at. This allows me to just do global.saplings[game.tick] and get all the needed saplings on that tick. What "idea 2" was supposed to be was an attempt to make it so planters could be stored back in the old method: ordered and contiguous table that could be looped over with the original method (see OP post). This would avoid next completely and prevent this from even being an issue. But that's ok. I've got this solved now by making a copy of the table and next looping over the copy instead of the original.
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

robot256
Filter Inserter
Filter Inserter
Posts: 594
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Spreading a table out over several ticks

Post by robot256 »

Is there a reason you are using next() instead of pairs()? There may not be any difference, and I don't think it solves the problem of adding to the list while iterating it, but the devs prefer using pairs() when iterating over a list with non-contiguous keys like unit numbers.

Making a copy of the table could start to be a UPS hog if someone puts down a million planters. If that happens, iterating over the main list and then putting any new entries in a temporary "things_to_add_later" list would speed it up.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

robot256 wrote:
Thu May 05, 2022 3:11 am
Is there a reason you are using next() instead of pairs()? There may not be any difference, and I don't think it solves the problem of adding to the list while iterating it, but the devs prefer using pairs() when iterating over a list with non-contiguous keys like unit numbers.
Yes, I'm not iterating over the full table each tick. See DaveMcW's post above (1st reply) on their method. I modified it slightly so it's still spreading all entities over X ticks, rather than X entities per tick.
robot256 wrote:
Thu May 05, 2022 3:11 am
Making a copy of the table could start to be a UPS hog if someone puts down a million planters. If that happens, iterating over the main list and then putting any new entries in a temporary "things_to_add_later" list would speed it up.
Unfortunately, it's the act of actually iterating over the main table when an add event gets called (which is completely separate from my looping of the table) that triggers the error. But if that's the case, the only other idea I can come up with is setting of a variable to true when the add event is called, and checking that var's state on each iteration of the loop, breaking if true (and then setting back to false for the next tick).
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

robot256
Filter Inserter
Filter Inserter
Posts: 594
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Spreading a table out over several ticks

Post by robot256 »

[Deleted previous comment once I read how next(table, index) works]

I just reread my similar code on Multiple Unit Train Control. Turns out, I use on_nth_tick to periodically copy the list of all things to update into a queue. Then on_tick removes one thing from the queue every tick and processes it, and turns off on_tick when the queue is empty. There is some fancy code to make on_nth_tick fire less often as the list if things expands.

So that technique is both similar to what you are doing, and not what you need. Glad you found something that works for you!
Last edited by robot256 on Thu May 05, 2022 4:42 am, edited 1 time in total.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

No, I don't do anything with multiplayer. :/ I am, however, storing its current pointer in the global table each loop (global.planter_index).

Here's the actual loop in question as it currently stands:

Code: Select all

local update_rate = math.ceil(global.planters_count / settings.global["treesaplingsredux_updateinterval"].value)
local planters = table.deepcopy(global.planters)
for i = 1, update_rate do
    local index = global.planter_index
    global.planter_index = next(planters, global.planter_index)
    processPlanter(planters[index])
end
The spreading out of the entities over X ticks isn't perfect, especially when there are only a few entities in the table, but I figure the impact from that should be negligible (until the number of entities in table >= updateinterval, it should only do 1 entity per tick, anyway).
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

robot256
Filter Inserter
Filter Inserter
Posts: 594
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Spreading a table out over several ticks

Post by robot256 »

I see. But if you make a new copy of the table every tick, wouldn't it have the same problem if the global.next_index you saved last time wasn't there anymore when you copy global.planters again the next tick? I assumed you were saving the copy in global until you finished looping all the way through it, then get a fresh copy from the list that has additions and deletions.

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

So far in my testing, no. If the current stored index no longer exists, then when I use it to pass to processPlanter, it just passes a nil value (and I then check for that first thing and return if nil). Next itself seems to be able to take an invalid index (even nil) and simply gives me another index. It may start over from the beginning, I’m unclear on that, but as this should only happen when the player (or biters :P ) remove a planter, I’m not too worried. The only reason for processing the planters, really, is to monitor their crafting progress and spawn a sapling at their position when they get above x%, and remove the sapling if they get removed before the sapling finishes or they hit 100% crafting progress and the sapling is still there (though that one shouldn’t trigger normally as I’ve purposely set craft time to longer than the max grow time). Everything else is technically handled by the sapling itself.

All of that said, I still need to do more testing to be sure. I have a couple blueprints ready for testing them that have thousands of planters in them. Old generation of code, I noticed that at ~2k planters with ~1.5k saplings growing I was taking a 5-10 ups hit. Since I changed the way saplings are processed, that same setup no longer has a ups hit. I want to build until I get a hit again to see the new limit, but time hasn’t been on my side, yet. :) And I’m also working on another method that takes out looping the planters altogether, but has some trade-offs (no randomized grow times, saplings instantly appear the moment the planter starts and can’t be mined or killed separately anything (including pollution)).
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

FuryoftheStars
Smart Inserter
Smart Inserter
Posts: 2485
Joined: Tue Apr 25, 2017 2:01 pm
Contact:

Re: Spreading a table out over several ticks

Post by FuryoftheStars »

FuryoftheStars wrote:
Thu May 05, 2022 12:25 pm
I noticed that at ~2k planters with ~1.5k saplings growing I was taking a 5-10 ups hit.
Whoops. Meant ~4k planters and ~3k-3.5k saplings.
My Mods: Classic Factorio Basic Oil Processing | Sulfur Production from Oils | Wood to Oil Processing | Infinite Resources - Normal Yield | Tree Saplings (Redux) | Alien Biomes Tweaked | Restrictions on Artificial Tiles

Qon
Smart Inserter
Smart Inserter
Posts: 2091
Joined: Thu Mar 17, 2016 6:27 am
Contact:

Re: Spreading a table out over several ticks

Post by Qon »

FuryoftheStars wrote:
Thu May 05, 2022 3:04 pm
FuryoftheStars wrote:
Thu May 05, 2022 12:25 pm
I noticed that at ~2k planters with ~1.5k saplings growing I was taking a 5-10 ups hit.
Whoops. Meant ~4k planters and ~3k-3.5k saplings.
Use the edit button.

Post Reply

Return to “Modding help”