Improper science pack removal in sctmlib.lua

[jacobwatkinsgit]

An error can be caused by science packs being removed with table.remove, when they are not whole number keys.

This is the original section, in my version, line 17 function function sctm.lab_input_remove(labname, packname):

   if data.raw.lab[labname] and data.raw.lab[labname].inputs then
        local labinputs = data.raw.lab[labname].inputs
        for _i, inputpack in pairs(labinputs) do
            if inputpack and inputpack == packname then
                table.remove(labinputs, _i)
                removed = true
                break
            end
        end
    end

This is a small tweak to fix it:

   if data.raw.lab[labname] and data.raw.lab[labname].inputs then
        local labinputs = data.raw.lab[labname].inputs
        for _i, inputpack in pairs(labinputs) do
            if inputpack and inputpack == packname then
                --table.remove(labinputs, _i)
                labinputs[_i] = nil
                removed = true
                break
            end
        end
    end

The inputs table of a lab is not required to be an array. It is explicitly listed as a "table of string" : https://wiki.factorio.com/Prototype/Lab#inputs

In the mod Mobile Factory, I wrote this section for a modified lab (data-final-fixes.lua):

    for k, lab in pairs(data.raw.lab) do
        for k2, name in pairs(lab.inputs) do
            inputsTable[name] = name
        end
    end
    data.raw.lab.DimensionalLab.inputs = inputsTable

This code caused the issue with your loop (string keys). I do not intend to sound impolite; your code is in err.

[jacobwatkinsgit]

Sorry for the repeated edits, was trying to fix the indentation.

[mexmer]

your assumption that _i is number is wrong, pairs return "index" and "object", so _i is index, it can be either numeric or string or anything else. but problem with your code is different, since your assignment effectively converts right side of pair to object.

also setting input[_i] to nil will create hole in inputs table, which causes different issue in other mods, and sometimes even in base, so this approach is wrong. what needs to be done is not to test name, but to also test _i, then it should work.

give me link to your mod, i will check it out, when i have time.

btw. my i ask, why you even do such thing? where you got this idea? because it doesn't make sense. it would make sense, if rightside of your assignment was something complex, but it is not.

not to mention, it must break more mods, than just mine.

[jacobwatkinsgit]

inputs is not required to be an array. Let me re-emphasize, table.remove(labinputs, _i)only works when _i is a number. inputs is explicitly a "table of strings". Again, https://wiki.factorio.com/Prototype/Lab#inputs .

This is a valid inputs table, as described by the lab prototype page, and by the fact that it loads and works:

  17.809 Script @__Mobile_Factory__/data-final-fixes.lua:15: {
  DimensionalCrystal = "DimensionalCrystal",
  DimensionalSample = "DimensionalSample",
  ["automation-science-pack"] = "automation-science-pack",
  ["chemical-science-pack"] = "chemical-science-pack",
  ["logistic-science-pack"] = "logistic-science-pack",
  ["military-science-pack"] = "military-science-pack",
  ["production-science-pack"] = "production-science-pack",
  ["space-science-pack"] = "space-science-pack",
  ["utility-science-pack"] = "utility-science-pack"
}

There is only one restriction on the inputs table, that a science pack does not occur twice or it results in a prototype error (that restriction is not listed on the wiki).

The mod with my code is here: https://mods.factorio.com/mod/Mobile_Factory .

Edit: I have not seen any other mod issues, and I've tried out Space Extension, Space Exploration, seablock+angels stuff, etc.

[mexmer]

your input is not table of strings, it's object.

what you created is

inputs {
 DimensionalCrystal : "DimensionalCrystal",
 DimensionalSample : "DimensionalSample",
 automation-science-pack : "automation-science-pack"
....
}

you can also access name of your pack by calling inputs.packname, and it will return string packname

this is far from specification. why it works i have no idea, but you can discuss it with any lua programmer, and he will tell you, this is not array of strings

but from your code, it's obvious, that i need to test packname.packname ~= nil and packname.packname = nametoremove then test will be matched, and table.remove(_i) will work as expected. or now when i think about it, easier test will be just _i = packname, because in your array, on index value is value of what needs to be removed.

problem is not in remove function of my code, problem is in test, which doesn't work with your implementation.

[mexmer]

@jacobwatkinsgit why use two different approaches to make inputs table in same code?

https://github.com/Mugiwaxar/Mobile-Factory/blob/master/data-final-fixes.lua

if "all" is enabled, you use named indexes (that causes problem with sctm, and is wrong approach, there is no reason to do this), and when "vanilla" is set, you use correct method to insert names of science packs to input table ...

what you should do (in "all") is use

table.insert(inputsTable, name)

at https://github.com/Mugiwaxar/Mobile-Factory/blob/1e7de7560a97cfbe0cb8cf16d9e53a5aef4a1ec9/data-final-fixes.lua#L11

you use table.insert (proper method) at https://github.com/Mugiwaxar/Mobile-Factory/blob/1e7de7560a97cfbe0cb8cf16d9e53a5aef4a1ec9/data-final-fixes.lua#L19

so your code also lack consistency in data manipulation.

[mexmer]

you will need to fix this on your side, this output is totally wrong

this is input table from serpent.block function (which most people use to log data, it's part of factorio mod engine)

log(serpent.block(data.raw.lab["DimensionalLab"].inputs))

output

{
  DimensionalCrystal = "DimensionalCrystal",
  DimensionalSample = "DimensionalSample",
  ["advanced-logistic-science-pack"] = "advanced-logistic-science-pack",
  ["automation-science-pack"] = "automation-science-pack",
  ["chemical-science-pack"] = "chemical-science-pack",
  ["logistic-science-pack"] = "logistic-science-pack",
  ["military-science-pack"] = "military-science-pack",
  ["production-science-pack"] = "production-science-pack",
  ["space-science-pack"] = "space-science-pack",
  ["utility-science-pack"] = "utility-science-pack"
} 

for comparison, this is inputs for bobs lab-2

log(serpent.block(data.raw.lab["lab-2"].inputs))

output

{
  "automation-science-pack",
  "logistic-science-pack",
  "chemical-science-pack",
  "military-science-pack",
  "production-science-pack",
  "advanced-logistic-science-pack",
  "utility-science-pack",
  "space-science-pack"
} 

[jacobwatkinsgit]

My code works and obeys Factorio's description of what the inputs must be. You are trying to force it to be an array of strings, which is not a requirement. I make a table of inputs by name so when I check every lab for inputs, I only have unique values. If I were to write the code how you want, I would need to have my existing table loop for unique science packs, AND another loop to insert WHEN THAT IS NOT A REQUIREMENT OF INPUTS. If it were a requirement, the wiki should say array of strings and the game would fail to load my inputs table. The wiki does not say array of strings and the game reads my table of strings without error. You are arguing that the game and wiki are wrong, not me.

[mexmer]

guess you missed why i was pointing it out.

i will do the fix, because factorio indeed accepts your input ... but if nothing else, you should be consistent in how you handle data and not use two different methods for same input.

again look at output of serpent.block function, and think about how it looks, if nothing else, make your data consistent, this way is hard to read or check anything.

so i retract my statement about "you need to fix your code", but still you should to be consistent, now your table is mess. if you want to stick with "text" index, ok, but make all indexes same looking

[mexmer]

this was fixed in 0.18.4 and 1.1.1 version

[jacobwatkinsgit]

Sorry if I sounded mean. The code style is just what is simplest at the time. unique-pack table -> write whole table as it is, only making additions (I know the packs are not there, and existing packs are like array) -> table.insert. I agree it may look inconsistent in one way, but to me it is more consistent than making them both as table/dictionary or both as arrays.