[Solved] various themes: comparing tables, general code quirks, blueprint modding and how not to behave as modder(sorry)

Place to get help with not working mods / modding interface.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

[Solved] various themes: comparing tables, general code quirks, blueprint modding and how not to behave as modder(sorry)

Post by Lindor »

Hi all :)
There is a really annoying issue for me: everytime i try to save something about entities and then the entity gets destroyed every table and variable referring to that entity gets reset, even globals.

E.g. having a for loop around a table of entities and one of the entities gets destroyed, the table gets the value of that entity removed and the for loop starts completely from the beginning regardless of where it was, running the body on some of the entities twice. I will use chests as an example. Something as simple as that:

Code: Select all

script.on_init(function()
local chests = game.surfaces[1].find_entities_filtered{name = {"logistic-chest-requester", "logistic-chest-passive-provider"}}
local i=0
    for k, v in pairs(chests) do
        if v.name == "logistic-chest-passive-provider"
            i = i+1
        end
        if v.name == "logistic-chest-requester"
            v.destroy()
        end
    end
game.surfaces[1].print(i)
end)
This will give the amount of passive provider chests in the table chests. BUT if even a single requester chest is in chests it will destroy it thus removing it from chests and starting the loop from the beginning which makes every passive provider chest coming before the requester chest counted twice.

Or having a local save a chests inventory (on init! the local isn't touched, only read i promise!) to insert its stacks into another inventory later. Once the chest gets destroyed, doesn't matter wether by being damaged, mined or via a script, the local gets reset making saves impossible. For globals (variables and the table) its the same.

Code: Select all

local chest = game.surfaces[1].find_entity("logistic-chest-requester", pos1)
local newchest = game.surfaces[1].find_entity("logistic-chest-requester", pos2)
local invtry = chest.get_inventory(1)
chest.destroy()
for u=1, #invtry do
    if invtry[u].valid_for_read == true then
        newchest.insert({name = invtry[u].name, count = invtry[u].count})
    end
end
After chest gets destroyed invtry will become an empty table making the for loop return the error "LuaInventory called when LuaInventory not valid". I think you get the idea.

How else am i supposed to lets say replace a requester chest with a buffer chest if i can't save the request slots in the script? Do i really have to create a "bridge-entity" e.g. a requester chest on a different surface just to temporarily save the properties i want to have? I hope that this is not supposed to be the case because it's really annoying. I've done a lot of testing on that because i couldn't believe it at first and thought i was simply a bad programmer and i'm still not convinced, but as much testing as i did i doubt that as well. So contradictory.

Now i have so many questions. Can you confirm that? Can you repeoduce? If not, can one of your friends? Is it supposed to be this way? Is there an error in my game files? Or am i just not good enough to write working scripts? If there is really a bug, is the reason for all these errors really that the variables get reset? Or did i misinterpret and there's another bug going on? Was it correct to post it in discussions since i don't need help with a specific mod? Or should i have posted it in help since i couldn't solve this myself?

And everybody who read this wall of thext thanks for your patience.
Last edited by Lindor on Sat Oct 05, 2019 6:16 am, edited 2 times in total.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

When i have problems i ask in "modding help" not "disucssion".

The behavior you're describing sounded like a bug in pairs() at first, factorio uses a custom implementation of pairs(), but such an obvious bug would surely have been reported long ago...

And your first example code works fine for me and counts the expected number of passive providers (after fixing the missing "then"'s.

Code: Select all

/c
local chests = game.surfaces[1].find_entities_filtered{name = {"logistic-chest-requester", "logistic-chest-passive-provider"}}
local i=0
    for k, v in pairs(chests) do
        if v.name == "logistic-chest-passive-provider" then
            i = i+1
        end
        if v.name == "logistic-chest-requester" then
            v.destroy()
        end
    end
game.surfaces[1].print(i)
Concerning the transfer of items code: If you destroy the chest then ofc the inventory is also destroyed. Why do you even *want* to destroy the chest *before* transferring items? Just destroy it after the transfer is done. No need to temporary store stuff in a third chest. Also your method will destroy all additional properties of transferred items (i.e. half used science packs, damaged walls, modular armor with equipment inside, etcpp). You should use LuaItemStack.transfer_stack() instead.

If the target inventory is not guaranteed to have the same or more slots then you need to deal with that too. You can call LuaInventory.sort_and_merge() but that won't help if the source is filled to the brim.

If the target is freshly created by script you might even try fast_replace + teleport.

Can't really help you further as long as i don't know what you want to do.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

Hello, thanks a lot, i really hope you or somebody else can help me.
I agree now that this is more of a help topic and i should have posted it there. Can maybe moderator move it there?

I've done some more research and managed to answer some of my questions. I've found that these are actually three distinct problems of which the last one really is a bug if i'm right.

Nr. 1: For the first code i've posted my testing has the same result as yours. Indeed it works fine (if you don't forget the "thens" like i did lol), i had named two variables the same in my first tests and didn't notice.

Nr. 2: For the second code i will explain in a second why i specifically want to destroy the entity first. But first let me explain what i've found out: If i interpret it right the second code is not working because LuaEntities are both an ingame entity and a table. If you save properties to a var it permanently saves the table and temporarily saves the entity only as long as it's a real entity in the game. It's the same for any other class. Which i guess makes sense because applying API commands to classes that are not actually there ingame can probably crash the game. But there is a really easy workaround: Simply do the commands before you destroy it and save the return in a table. To give an example this is how i modified my second code to make it work:

Code: Select all

function invtransfer(fromchest, tochest)
    local invtry = {{}}
    local invtrysize = #fromchest.get_inventory(1)
    local validslots = {}
    for u=1, invtrysize do
        if fromchest.get_inventory(1)[u].valid_for_read == true then
            invtry[u] = {fromchest.get_inventory(1)[u].name, fromchest.get_inventory(1)[u].count}
            validslots[u] = u
        end
    end
    fromchest.destroy()
    for k, v in pairs(validslots) do
        if invtry[v][1] ~= nil then --just to be safe
            tochest.get_inventory(1)[v].set_stack({name = invtry[v][1], count = invtry[v][2]})
        end
    end
end
Should work fine.

The reason why i want to destroy the chest first is i'm currently working on a mod i called overflow chests. For long time there were suggestions in the forums to merge all the different logistic chests into one entity. I want to merge passive provider and requester chests by making the chest a provider when the request is satisfied and back to requester when it drops below the request again. It would solve some a lot of problems like bots taking items always from the same chest instead of evenly distributing their takings over the neirer chests. Since we can't change the logistic mode via API i solve the problem by creating two different entities with the same ingame name and replacing each other depending on their inventory ingame. Therefore i need of course to save the request slots of overflow chests without having the ingame entity so i save it with the position instead.

Nr. 3: This leads to the third problem which is a bug in my opinion. This is my code:

Code: Select all

--setting up the globals. We need all modded chests and their request slots.
script.on_init(function()
    if global.reqtable == nil then
        global.reqtable = {{{1}}}
    end
    global.chests = game.surfaces[1].find_entities_filtered{type = "logistic-container", name = {"logistic-chest-overflow", "logistic-chest-overflow-provider"}}
end)


--replace a chest with another and keep the inventory. It's a lot bette than fast replace because we get rid of the mined result. Only switch if the chest not already is what we need.
function switchchests(fromchest, fromname, toname, frompos)
    if fromchest.valid == true then
        if fromchest.name == fromname then
    	    local invtry = {{}}
            local invtrysize = #fromchest.get_inventory(1)
            local validslots = {}
            for u=1, invtrysize do
                if fromchest.get_inventory(1)[u].valid_for_read == true then
                    invtry[u] = {fromchest.get_inventory(1)[u].name, fromchest.get_inventory(1)[u].count}
                    validslots[u] = u
                end
            end
            fromchest.destroy({do_cliff_correction=false, raise_destroy=true})
            game.surfaces[1].create_entity{name=toname, position=frompos, force="player", fast_replace=false, raise_built=true}
            for k, u in pairs(validslots) do
                if invtry[u][1] ~= nil then
                    game.surfaces[1].find_entity(toname, frompos).get_inventory(1).insert({name = invtry[u][1], count = invtry[u][2]})
                end
            end
        end
    end
end


-- the main logic
function main(event)
     --every 30 ticks which means twice a second
    if event.tick % 30 == 0 then
        local locreqtable = global.reqtable
        --check all chests
        for k, chest in pairs(global.chests) do
            if chest.valid == true then
                local pos = {chest.position.x, chest.position.y}
                local i = 0
                local request = {{nil}}
                if locreqtable[pos] ~= nil then
                    game.surfaces[1].print("a")
                end
                for value = 1, 12 do
                    if chest.name == "logistic-chest-overflow" then
                        if chest.get_request_slot(value) ~= nil then
                            request[value] = {chest.get_request_slot(value).name, chest.get_request_slot(value).count}
                            -- We need the request slots even without having the entity so they need to be saved with the position instead.
                            locreqtable[pos] = request
                        end
                    end
                end
                for value = 1, 12 do
                    if locreqtable[pos] ~= nil then
                        if locreqtable[pos][value] ~= nil then
                            if locreqtable[pos][value][1] ~= nil then
                                --look wether the request has been satisfied or not
                                if locreqtable[pos][value][2] <= chest.get_item_count(locreqtable[pos][value][1]) then
                                    i = 1
                                end
                            end
                        end
                    end
                    --if it has been satisfied, switch it to a provider chest or end
                    if i == 1 then
                        do switchchests(chest, "logistic-chest-overflow", "logistic-chest-overflow-provider", pos) end
                    end
                    --if not, switch it back or end
                    if i == 0 then
                        do switchchests(chest, "logistic-chest-overflow-provider", "logistic-chest-overflow", pos) end
                    end
                end
                if locreqtable[global.tatata] ~= nil then
                    game.surfaces[1].print("b")
                end
            end
        end
        global.reqtable = locreqtable
    end				
end

script.on_event({defines.events.on_tick}, main)


--check for new chests
function built(event)
    local entity = event.entity or event.created_entity
        if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
        table.insert(global.chests, entity)
    end
end
	
script.on_event(defines.events.on_built_entity, built)
script.on_event(defines.events.on_robot_built_entity, built)
script.on_event(defines.events.script_raised_built, built)


--check for deleted chests. Because table.remove() doesn't work if you don't know the key we need to instead insert everything but the removed chests in a new table.
function mined(event)
    local entity = event.entity
    local bridgetable = {}
    if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
        for key, ent in pairs(global.chests) do
            if ent ~= entity then
                table.insert(bridgetable, ent)
            end
        end
    end
    global.chests = bridgetable
end

script.on_event(defines.events.on_player_mined_entity, mined)
script.on_event(defines.events.on_robot_mined_entity, mined)
script.on_event(defines.events.script_raised_destroy, mined)
The problem lives in line 41:

Code: Select all

for k, chest in pairs(global.chests) do
if you plant a single chest ingame and give it a request right before that line global.reqtable has the request slots saved with the position as key. Right after the line global.reqtable gets completely reset and cleared which results that ingame you get constantly printed "b" in your console, but never "a". You can check in your own game: just copy this code in a control.lua, replace "logistic-chest-overflow" with "logistic-chest-requester" and "logistic-chest-overflow-provider" with "logistic-chest-passive-provider", start a new game, place a single requester chest and give it a request.

I think i might even have a reason for that bug. In the wiki it says that the devs do everything to make sure that if you want to save data with an entity, you have to do it in your global table. The reason is that there the game has control about everything saved in contrast to your variables in the script. The devs do everything to make sure you don't do some fancy stuff that could lead to crashes and desyncs like saving something with entities that are not ingame anymore. Remember how i said that entites are both a LuaEntity and table? Unfortunately the algorithms that are supposed to save us can not distinguish that thus deleting both. The same goes for positions, surfaces, itemstacks and every other lua class.That is at least what i believe is going on here.

I hope that someone might find a workaround that helps me finishing this mod but this seriously needs to be fixed.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Tue Oct 01, 2019 2:18 am That is at least what i believe is going on here.

I hope that someone might find a workaround that helps me finishing this mod but this seriously needs to be fixed.
Well, let me first be the "bad cop" and tell you that nothing you describe needs to be fixed, you simply have an insufficient understanding of the api and lua due to your inexperience. Inexperience is fine, we all start somewhere, but assuming that everything that doesn't work as you expected is "broken" will hamper your progress. When you're new to anything it's usually safe to assume that everything you don't understand is your fault, not everyone elses.
Lindor wrote: Tue Oct 01, 2019 2:18 am --replace a chest with another and keep the inventory. It's a lot bette than fast replace because we get rid of the mined result. Only switch if the chest not already is what we need.
You can get rid of the mined result with LuaSurface.create_entity{fast_replace=true,player=nil,spill=false}. And fast replace suffers none of the item attribute bugs you're causing (as mentiond in previous post).
Lindor wrote: Tue Oct 01, 2019 2:18 am The reason why i want to destroy the chest first is i'm currently working on a mod i called overflow chests.
I see no reason here. Maybe you don't know that two entites can exist at the same position?
Lindor wrote: Tue Oct 01, 2019 2:18 am local invtry = {{}}
global.reqtable = {{{1}}}
local request = {{nil}}
Why are you filling the first table element with an empty table just to override it during the next loop? If you need an empty_table = {} you should do exactly that.
Lindor wrote: Tue Oct 01, 2019 2:18 am has the request slots saved with the position as key
The position is returned as a table. Tables in lua have a unique identity that is compared when you compare two tables. If you want to compare the content of a table you need to write a function for this.

Code: Select all

/c game.print({} == {}) --[[false!]]
/c local a = {}; local b = a; game.print(a==b) --[[true]]
I recommend you use the LuaEntity.unit_number of the newly created passive provider as a key instead of the position. The unit_number is a unique identifier for each entity, so it'll change every time you create/destroy the chest. But as you only need to store the requests while a chest is in "passive provider mode" that should be fine.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

Oh you've done me such a big favor. For three days i've been trying to get this script working now. It looks a little bit messy now but at least it works as intended in singleplayer. I will definitely thank you in the mod description unless you don't want me to.
eradicator wrote: Tue Oct 01, 2019 8:47 am Why are you filling the first table element with an empty table just to override it during the next loop? If you need an empty_table = {} you should do exactly that.
That was just for my sanity. The whole table in a table in a table thing has confused me so much sometimes that i wanted some way to see how many keys i need to get to the final value.

The fast replace works great. It's probably not even worth defining a whole function for it anymore. I decided not to use a function to compare values of tables and instead went for your advice with the ID. Worked great so far. Here in case you're interested:

Code: Select all

--setting up the globals. We need all modded chests and their request slots.
script.on_init(function()
    if global.reqtable == nil then
        global.reqtable = {}
    end
    global.chests = game.surfaces[1].find_entities_filtered{type = "logistic-container", name = {"logistic-chest-overflow", "logistic-chest-overflow-provider"}}
end)


--fast replace a chest with another and delete the old one from global.chests We need to do this manually because create_entity won't call on_script_raised_destroy.
function switchchests(fromchest, toname, frompos, key)
    if fromchest.valid == true then
        global.chests[key] = nil
        game.surfaces[1].create_entity{name=toname, position=frompos, force="player", fast_replace=true, raise_built=true, player=nil, spill=false}
    end
end


-- the main logic
function main(e)
    --every 30 ticks which means twice a second
    if e.tick % 30 == 0 then
        --check all chests
        for k, chest in pairs(global.chests) do
            if chest.valid == true then
                local pos = {chest.position.x, chest.position.y}
                local i = 0
                local chestID = chest.unit_number
                if chest.name == "logistic-chest-overflow" then
                    for slot = 1, 12 do
                        if chest.get_request_slot(slot) ~= nil then
                            --has the request been satisfied?
                            if chest.get_request_slot(slot).count <= chest.get_item_count(chest.get_request_slot(slot).name) then
                                i = 1
                            end
                        end
                    end
                end
                if chest.name == "logistic-chest-overflow-provider" then
                    for slot = 1, 12 do
                        if global.reqtable[chestID] ~= nil then
                            if global.reqtable[chestID][slot] ~= nil then
                                --has the request been satisfied?
                                if global.reqtable[chestID][slot][2] <= chest.get_item_count(global.reqtable[chestID][slot][1]) then
                                    i = 1
                                end
                            end
                        end
                    end
                end
                --if it has been satisfied, switch it to a provider chest and save the request with unit_number or end
                if i == 1 then
                    if chest.name == "logistic-chest-overflow" then
                        local request = {}
                        for slot = 1, 12 do
                            if chest.get_request_slot(slot) ~= nil then
                                request[slot] = {chest.get_request_slot(slot).name, chest.get_request_slot(slot).count}
                            end
                        end
                        do switchchests(chest, "logistic-chest-overflow-provider", pos, k) end
                        global.reqtable[game.surfaces[1].find_entity("logistic-chest-overflow-provider", pos).unit_number] = request
                    end
                end
                --if not, switch it back and set the saved slots again or end
                if i == 0 then
                    if chest.name == "logistic-chest-overflow-provider" then
                        do switchchests(chest, "logistic-chest-overflow", pos, k) end
                        local newchest = game.surfaces[1].find_entity("logistic-chest-overflow", pos)
                        for slot = 1, 12 do
                            if global.reqtable[chestID] ~= nil then
                                if global.reqtable[chestID][slot] ~= nil then
                                    newchest.set_request_slot({name=global.reqtable[chestID][slot][1], count=global.reqtable[chestID][slot][2]}, slot)
                                end
                            end
                        end
                        --delete the saved requests to prevent reqtable from getting massive
                        global.reqtable[chestID] = nil
                    end
                end
            end
        end
    end				
end

script.on_event({defines.events.on_tick}, main)


--check for new chests
function built(event)
    local entity = event.entity or event.created_entity
    if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
        table.insert(global.chests, entity)
    end
end
	
script.on_event(defines.events.on_built_entity, built)
script.on_event(defines.events.on_robot_built_entity, built)
script.on_event(defines.events.script_raised_built, built)


--check for deleted chests. Because table.remove() doesn't work if you don't know the key we need to instead insert everything but the removed chests in a new table.
function mined(event)
    local entity = event.entity
    local bridgetable = {}
    if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
        for key, ent in pairs(global.chests) do
            if ent ~= entity then
                table.insert(bridgetable, ent)
            end
        end
    end
    global.chests = bridgetable
end

script.on_event(defines.events.on_player_mined_entity, mined)
script.on_event(defines.events.on_robot_mined_entity, mined)
script.on_event(defines.events.script_raised_destroy, mined)
Hopefully i can submit soon.
And finally whoever you are, thanks for moving the topic in help ;).
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

After a quick glance at your code i found some more quirks (i'm sure i didn't find everything).

Code: Select all

game.surfaces[1]
Very bad style, you should never use hardcoded indexes. It'll crash with tons of mods, i.e. factorissimo2, space exploration to name just two popular ones. Grab the surface from the entity you're dealing with: LuaEntity.surface
________________

Code: Select all

do switchchests(chest, "logistic-chest-overflow-provider", pos, k) end
global.reqtable[game.surfaces[1].find_entity("logistic-chest-overflow-provider", pos).unit_number] = request
create_entity *returns* the entity, there's no need to search it. so this:

Code: Select all

function switchchests(fromchest, toname, frompos, key)
    if fromchest.valid == true then
        global.chests[key] = nil
        --[[return the entity!]]
        return game.surfaces[1].create_entity{name=toname, position=frompos, force="player", fast_replace=true, raise_built=true, player=nil, spill=false}
    end
end

local newchest = switchchests(chest, "logistic-chest-overflow-provider", pos, k)
global.reqtable[newchest.unit_number] = request
________________

Code: Select all

if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
This is *always* true. "or" does not work magically like that. The second string is not compared it's checked for existance.

What you wrote:

Code: Select all

if (entity.name == "logistic-chest-overflow") or (nil =~ "logistic-chest-overflow-provider") then
What you meant:

Code: Select all

if (entity.name == "logistic-chest-overflow") or (entity.name == "logistic-chest-overflow-provider") then
________________

Code: Select all

function mined(event)
    local entity = event.entity
    local bridgetable = {}
    if entity.name == "logistic-chest-overflow" or "logistic-chest-overflow-provider" then
        for key, ent in pairs(global.chests) do
            if ent ~= entity then
                table.insert(bridgetable, ent)
            end
        end
    end
    global.chests = bridgetable
end
Why reconstruct the whole table? Just remove the one thing that was destroyed.

Code: Select all

for key,ent in pairs(global.chests) do
  if ent == entity then 
    table.remove(global.chests,key)
    break
    end
  end
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

Your tips are so helpful. I implemented each of your suggestions. Hopefully i will be able to spot these things myself one day.
Lindor wrote: Tue Oct 01, 2019 4:24 pm Hopefully i can submit soon.
Seems like i was way too enthusiastic here. Ingame testing showed me some problems that i never thought about. There's still a lot of work to do, especially for blueprints. But i have some ideas that are somewhat hard to explain with words for me, maybe i am able to do the first basic scripts today then i will show them.

But i have a question: For a chest how can i get the items that are being fetched by the bots? Couldn't find anything in the lua-api. I need to take those into the equation or else the chests will create a lot of unnecessary trips and logistic loops for the bots.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 9:50 am For a chest how can i get the items that are being fetched by the bots?
Not really sure what you mean by that. You can't control the bots, so if fast_replacing a chest causes them to cancel their current "job" there's nothing you can do about it. Honestly i don't really understand how switching between requester/provider chests is different from just using a buffer chest either :p.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

eradicator wrote: Wed Oct 02, 2019 10:00 am Not really sure what you mean by that.
This:
(bottom right)
Image
eradicator wrote: Wed Oct 02, 2019 10:00 am You can't control the bots, so if fast_replacing a chest causes them to cancel their current "job" there's nothing you can do about it.
Well, i can't control them individually but i can control the behavior of the chests. The problem is that bots take items from the chest despite being in requester mode. I want to switch the chests back to requester mode earlier, when the difference between the inventory items and the items that are being fetched reaches the request condition.
eradicator wrote: Wed Oct 02, 2019 10:00 amHonestly i don't really understand how switching between requester/provider chests is different from just using a buffer chest either :p.
The main point is that bots always take items from the nearest available chest instead of evenly distribute their transports which results in half the provider chests being full and half being completely empty. I want that all smelters and factories produce instead of just the nearest. If there are production pauses the chests will refill faster.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 1:09 pm
eradicator wrote: Wed Oct 02, 2019 10:00 am Not really sure what you mean by that.
This:
(bottom right)
Image
I don't see any way to access that in the apidoc for LuaEntity, LuaLogisticCell or LuaLogisticNetwork. So i strongly suspect that it's not currently possible. Not sure if that's a realistic thing to make a request for.
Lindor wrote: Wed Oct 02, 2019 1:09 pm Well, i can't control them individually but i can control the behavior of the chests. The problem is that bots take items from the chest despite being in requester mode.
So you're saying if a bot is on its way to fetch stuff from a passive provider and you fast_replace that with a requester, the bot will then still take items out of the requester (once)?
Lindor wrote: Wed Oct 02, 2019 1:09 pm The main point is that bots always take items from the nearest available chest instead of evenly distribute their transports which results in half the provider chests being full and half being completely empty. I want that all smelters and factories produce instead of just the nearest. If there are production pauses the chests will refill faster.
I can understand that from a "it would look more pleasing" way, but from the raw production numbers it shouldn't matter. If you're overproducing some of the furnaces will stay idle (saves UPS) and if you're underproducing eventually all the chests will be completely empty. Could also be solved with active providers limited to one inventory slot + buffer chests between the furnaces and wherever the stuff needs to go.
But that also means that you essentially want to switch passive providers "on/off" and you're just abusing requester chests to achieve that? In which case i'd suggests trying to LuaInventory.setbar() to limit the amount of stuff robots are allowed to take out of each chest (the red cross in the lower right of every chest).
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

eradicator wrote: Wed Oct 02, 2019 1:40 pm I don't see any way to access that in the apidoc for LuaEntity, LuaLogisticCell or LuaLogisticNetwork. So i strongly suspect that it's not currently possible. Not sure if that's a realistic thing to make a request for.
:cry:
eradicator wrote: Wed Oct 02, 2019 1:40 pmSo you're saying if a bot is on its way to fetch stuff from a passive provider and you fast_replace that with a requester, the bot will then still take items out of the requester (once)?
Yes
eradicator wrote: Wed Oct 02, 2019 1:40 pm I can understand that from a "it would look more pleasing" way, but from the raw production numbers it shouldn't matter. If you're overproducing some of the furnaces will stay idle (saves UPS) and if you're underproducing eventually all the chests will be completely empty. Could also be solved with active providers limited to one inventory slot + buffer chests between the furnaces and wherever the stuff needs to go.
But that also means that you essentially want to switch passive providers "on/off" and you're just abusing requester chests to achieve that? In which case i'd suggests trying to LuaInventory.setbar() to limit the amount of stuff robots are allowed to take out of each chest (the red cross in the lower right of every chest).
I've also thought about this for a while. You're saying the same things i think. for a balanced factory it shouldn't matter because ideally you're producing exactly as much as you're taking. It's not a feature to give us better production statistics. It's a creative feature. I just really want it to be that way, don't know why :mrgreen: .

Each of the ideas has it's own cons, with active providers you can't have empty storages for getting rid of trash or you need them to be in a different network, bars also limit the amount of stuff that can be put in and something like a shared inventory would just be too op, even with restrictions like "only bots can take items out" or something like that. I want to be able to have full chests and i want the ability to tell the game how full till to turn it off.

The two ideas that stood out to me were to either use connector logic (is it even called that in english?) or the mod that i try to create. The provider/requester switch seems to be what i think is a smarter way to do it beause it not only gives us new mechanics for provider chests but also for requester chests as a bonus. I don't know wether it's useful but it's there. :D
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 2:33 pm
eradicator wrote: Wed Oct 02, 2019 1:40 pm I don't see any way to access that in the apidoc for LuaEntity, LuaLogisticCell or LuaLogisticNetwork. So i strongly suspect that it's not currently possible. Not sure if that's a realistic thing to make a request for.
:cry:
Might still be buried somewhere. It's just that *i* am out of ideas where to look. (Changing the thread title to something more meaningful might lure in other people ;).
Lindor wrote: Wed Oct 02, 2019 2:33 pm It's a creative feature. I just really want it to be that way, don't know why :mrgreen: .
I certainly don't want to stop you from trying to mod it in. It's what i do to when i don't like something.
Lindor wrote: Wed Oct 02, 2019 2:33 pmshared inventory would just be too op
Shared inventories do not exist.
Lindor wrote: Wed Oct 02, 2019 2:33 pm Each of the ideas has it's own cons, with active providers you can't have empty storages for getting rid of trash or you need them to be in a different network,
Correct. That approach assumes that the furnaces are on a seperate network. I never liked "one network to rule them all" so i simply forgot to mention that ;). I'm just unsure how realistic your current approach is. What i meant with setbar() is that you could script/mod it to be dynamically used to "disable" a passive provider, which would be less destructive than replacing it. Setting it from circuit network might also be a viable approach if you can make it work (which is probably rather difficult to do nicely).
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

The problem with blueprints is that we can only allow chests in requester mode with it's request slots but theres no way to draw a connection between an entities unit number and it's blueprinted entity.
This is my idea (didn't really test yet):

Code: Select all

--check for new created blueprints
function blueprint(event)
    local tick = defines.events.on_tick
    local u = 0
    repeat
        if tick then
            u = u + 1
            if u % 1800 == 0 then
                goto stop
            end
        end
    until defines.events.on_player_configured_blueprint ~= nil
    local surf = event.player.surface
    local blueprov = surf.find_entities_filtered(type = "logistic-container", name ="logistic-chest-overflow-provider", area=event.area)
    for number, chest in pairs(blueprov) do
        local chestID = chest.unit_number
        local newchest = surf.create_entity{name="logistic-chest-overflow", position=chest.position, force="player", fast_replace=true, raise_built=true, player=nil, spill=false}
        for slot = 1, 12 do
            if global.reqtable[chestID] ~= nil then
                if global.reqtable[chestID][slot] ~= nil then
                    newchest.set_request_slot({name=global.reqtable[chestID][slot][1], count=global.reqtable[chestID][slot][2]}, slot)
                end
            end
        end
        global.reqtable[chestID] = nil
    end
    event.item.create_blueprint(surface=surf, force="player", area=event.area)
	::stop::
end

script.on_event.(defines.events.on_player_setup_blueprint, blueprint)
eradicator wrote: Wed Oct 02, 2019 2:55 pm Changing the thread title to something more meaningful might lure in other people ;)
Clickbait? ;) Well i guess i'm a youtuber now lol.

Edit:
eradicator wrote: Wed Oct 02, 2019 2:55 pm
Lindor wrote: Wed Oct 02, 2019 2:33 pmshared inventory would just be too op
Shared inventories do not exist.
Well i had the idea to constantly check for all chests items being inserted/taken and update them to reference chests inventory. Then every tick update the reference chests inventory items to every chests inventory. Would probably get extremely laggy but i think it's possible to fake a shared inventory.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 4:09 pm The problem with blueprints is that we can only allow chests in requester mode with it's request slots but theres no way to draw a connection between an entities unit number and it's blueprinted entity.
There will be in 0.17.70+. Edit: There is in 0.17.69+.
Lindor wrote: Wed Oct 02, 2019 4:09 pm This is my idea (didn't really test yet):

Code: Select all

--check for new created blueprints
function blueprint(event)
    local tick = defines.events.on_tick
    local u = 0
    repeat
        if tick then
            u = u + 1
            if u % 1800 == 0 then
                goto stop
            end
        end
    until defines.events.on_player_configured_blueprint ~= nil
Honestly...that code is so completely wrong i don't even know where to start explaining. Defines are constants. defines.events.on_tick is not the current tick, that would be event.tick. Events do not last several ticks. I have no clue what you're trying to count there with the repeat loop, but that thing will always count to 1800 and then "goto stop". Btw, *never ever* use "goto" for anything, it's an artifact of old times and will only cause you headaches. In that particular case it looks like you just want a plain "return", but again...no clue what the loop is supposed to do in the first place.
Lindor wrote: Wed Oct 02, 2019 4:09 pm
eradicator wrote: Wed Oct 02, 2019 2:55 pm Changing the thread title to something more meaningful might lure in other people ;)
Clickbait? ;) Well i guess i'm a youtuber now lol.
Other people know other things. And the title was non-descript *and* wrong before.
Last edited by eradicator on Wed Oct 02, 2019 4:34 pm, edited 2 times in total.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Bilka
Factorio Staff
Factorio Staff
Posts: 3310
Joined: Sat Aug 13, 2016 9:20 am
Contact:

Re: really annoying bug

Post by Bilka »

eradicator wrote: Wed Oct 02, 2019 4:24 pm
Lindor wrote: Wed Oct 02, 2019 4:09 pm The problem with blueprints is that we can only allow chests in requester mode with it's request slots but theres no way to draw a connection between an entities unit number and it's blueprinted entity.
There will be in 0.17.70+.
Isn't that already in 0.17.69?
0.17.69 changelog wrote:Added a blueprint entity index to source entity mapping to the on_player_setup_blueprint event.
There are no modding changes in 0.17.70 related to blueprints that I can see.
I'm an admin over at https://wiki.factorio.com. Feel free to contact me if there's anything wrong (or right) with it.
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Bilka wrote: Wed Oct 02, 2019 4:31 pm
eradicator wrote: Wed Oct 02, 2019 4:24 pm
Lindor wrote: Wed Oct 02, 2019 4:09 pm The problem with blueprints is that we can only allow chests in requester mode with it's request slots but theres no way to draw a connection between an entities unit number and it's blueprinted entity.
There will be in 0.17.70+.
Isn't that already in 0.17.69?
0.17.69 changelog wrote:Added a blueprint entity index to source entity mapping to the on_player_setup_blueprint event.
There are no modding changes in 0.17.70 related to blueprints that I can see.
Oh, you're right.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

eradicator wrote: Wed Oct 02, 2019 4:24 pmBtw, *never ever* use "goto" for anything, it's an artifact of old times and will only cause you headaches.
I wanted to trigger the old batch vibes xd The moment i wrote it down i knew you would complain.
eradicator wrote: Wed Oct 02, 2019 4:24 pmI have no clue what you're trying to count there with the repeat loop, but that thing will always count to 1800 and then "goto stop".
Exactly. I give the player 30 sec to confirm the blueprint. If he doesn't, end. If he does before 30 sec, continue.
eradicator wrote: Wed Oct 02, 2019 4:24 pm Honestly...that code is so completely wrong i don't even know where to start explaining.
Ok, ok, it was just a quickly written script to bring the idea across. I wanted you to understand what i try to do, thats everything.

Code: Select all

--check for new created blueprints
x=0

function blueprint(event)
    local tick = game.tick
    local u = 0
    local s = 0
    repeat
        if s = 0 then
            if tick ~= game.tick then
                u = u + 1
    	        if u % 1800 == 0 then
                    s = 1
                    break
                end
            end
        end
    until x == 1
    x = 0
    if s == 0 then
        local surf = event.player.surface
        local blueprov = surf.find_entities_filtered(type = "logistic-container", name ="logistic-chest-overflow-provider", area=event.area)
        for number, chest in pairs(blueprov) do
            local chestID = chest.unit_number
            local newchest = surf.create_entity{name="logistic-chest-overflow", position=chest.position, force="player", fast_replace=true, raise_built=true, player=nil, spill=false}
            for slot = 1, 12 do
                if global.reqtable[chestID] ~= nil then
                    if global.reqtable[chestID][slot] ~= nil then
                        newchest.set_request_slot({name=global.reqtable[chestID][slot][1], count=global.reqtable[chestID][slot][2]}, slot)
                    end
                end
            end
            global.reqtable[chestID] = nil
        end
        event.item.create_blueprint(surface=surf, force="player", area=event.area)
    end
end

function conf(event)
x = 1
end

script.on_event.(defines.events.on_player_setup_blueprint, blueprint)
script.on_event.(defines.events.on_player_configured_blueprint, conf)
Better?
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: really annoying bug

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 5:12 pm Better?
No. For the purpose of modding the game runs in a single thread. If you stall while counting to 1800 (which will take a few microseconds) the game does not continue. All events terminate in the tick they are raised. All events happen in order. Nothing else happens before your function doesn't return (unless you cause raising of more events inside an event).

I think what you want to do is (switch all chests to requester mode) -> recreate the blueprint -> (switch all chests to provider mode). You don't need to wait for the player to do anything once your settings are stored in the BP. Might have to force re-open (LuaPlayer.opened=) the blueprint window after re-creating the print, not sure. Might also poison the event for all mods coming after yours (@Bilka opinion/knowledge regarding the event.mapping when the BP is overwritten by stack.create_blueprint?). With the new event.mapping you don't even need to swap the chests, you can just fetch the data from your global table.
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Lindor
Inserter
Inserter
Posts: 30
Joined: Sat Sep 28, 2019 10:54 pm
Contact:

Re: really annoying bug

Post by Lindor »

eradicator wrote: Wed Oct 02, 2019 7:19 pm No. For the purpose of modding the game runs in a single thread. If you stall while counting to 1800 (which will take a few microseconds) the game does not continue. All events terminate in the tick they are raised. All events happen in order. Nothing else happens before your function doesn't return
Oh man i'm such a noob. It even stops forever since i'm counting on game ticks and there are no ticks anymore after the loop.
eradicator wrote: Wed Oct 02, 2019 7:19 pm I think what you want to do is (switch all chests to requester mode) -> recreate the blueprint -> (switch all chests to provider mode).
Yes but i don't need to switch them back cause i have the main functions body for that.
eradicator wrote: Wed Oct 02, 2019 7:19 pmMight have to force re-open (LuaPlayer.opened=) the blueprint window after re-creating the print, not sure. Might also poison the event for all mods coming after yours (@Bilka opinion/knowledge regarding the event.mapping when the BP is overwritten by stack.create_blueprint?). With the new event.mapping you don't even need to swap the chests, you can just fetch the data from your global table.
I really hoped to be able to avoid the force re-open of that because of the obvious reasons but there is another problem that is that requester and provider are two different entities but only the requester chest must be craftable. Cause the providers would never be able to be swapped cause they have no configurable request slots it would be pointless for them to be craftable. And there's no point in having uncraftable items in a blueprint. Making a completely pointless item craftable just for the script to work feels like inventing 7 new dimensions just for the math to work. I don't like it. If i want to use mapping i still need to swap the chests. I'd also really like to learn more, Bilka.
eradicator wrote: Wed Oct 02, 2019 7:19 pm (unless you cause raising of more events inside an event).
This gave me an idea:

Code: Select all

--check for new created blueprints
function blueprint(event)
    script.on_event(defines.events.on_player_configured_blueprint, function ()
        local surf = event.player.surface
        local blueprov =  surf.find_entities_filtered{type="logistic-container", name="logistic-chest-overflow-provider", area=event.area}
        for number, chest in pairs(blueprov) do
            local chestID = chest.unit_number
            local newchest = surf.create_entity{name="logistic-chest-overflow", position=chest.position, force="player", fast_replace=true, raise_built=true, player=nil, spill=false}
            for slot = 1, 12 do
                if global.reqtable[chestID] ~= nil then
                    if global.reqtable[chestID][slot] ~= nil then
                        newchest.set_request_slot({name=global.reqtable[chestID][slot][1], count=global.reqtable[chestID][slot][2]}, slot)
                    end
                end
            end
            global.reqtable[chestID] = nil
        end
        event.item.create_blueprint{surface=surf, force="player", area=event.area}
    )
end

script.on_event(defines.events.on_player_setup_blueprint, blueprint)
What do you think? Can this work? Unfortunately on_player_configured_blueprint does not contain the blueprint entity according to the lua-api. How can i make sure that setup-q-setup-confirm doesn't create bugs?
User avatar
eradicator
Smart Inserter
Smart Inserter
Posts: 5207
Joined: Tue Jul 12, 2016 9:03 am
Contact:

Re: How to get a logistic chests items being fetched?

Post by eradicator »

Lindor wrote: Wed Oct 02, 2019 9:41 pm Yes but i don't need to switch them back cause i have the main functions body for that.
Making blueprints should not affect how the factory works. But that's more of an ugly quirk than a bug if you want to do it like that.
Lindor wrote: Wed Oct 02, 2019 9:41 pm I really hoped to be able to avoid the force re-open of that because of the obvious reasons
Please state the nature of the medical emergency obvious reasons. I don't see any. Reopening a gui in the same tick it was closed is unnoticible™ for the player.
Lindor wrote: Wed Oct 02, 2019 9:41 pm but there is another problem that is that requester and provider are two different entities but only the requester chest must be craftable. Cause the providers would never be able to be swapped cause they have no configurable request slots it would be pointless for them to be craftable. And there's no point in having uncraftable items in a blueprint.
I thought the whole point of the blueprint hacking was to never allow them to be in a blueprint in the first place. So why worry about craftability? Are you unaware of the fact that you can change the type of the chest *inside* the blueprint (via get_blueprint_entities, set_blueprint_entities)? And then there's also "placeable_by" if for some reason you still need that.
Lindor wrote: Wed Oct 02, 2019 9:41 pm
eradicator wrote: Wed Oct 02, 2019 7:19 pm (unless you cause raising of more events inside an event).
This gave me an idea:
I'm already regretting i said that. It's a technical detail. You don't want to "use" that. It won't help you. And reassigning event handlers inside events will only affect the *next* time the event is raised anyway.
Lindor wrote: Wed Oct 02, 2019 9:41 pm How can i make sure that setup-q-setup-confirm doesn't create bugs?
By wanting to manipulate blueprints while the player makes them you've chosen a minefield as your first modding environment. You have to do a lot of learning and testing to get it right. And you'll have to do most of it on your own. (Also what is "setup-q-setup-confirm" supposed to mean? Did you misunderstand "re-open the gui" to be a player action?)
Author of: Belt Planner, Hand Crank Generator, Screenshot Maker, /sudo and more.
Mod support languages: 日本語, Deutsch, English
My code in the post above is dedicated to the public domain under CC0.
Post Reply

Return to “Modding help”