new data:extend

[Lindor]

Hello dear devs ,

Let's suppose you want to add an area-attack to data.raw["beam"]["laser-beam"]. You deepcopy the table and add your area attack to laserBeam.action. Now if for some reason future vanilla factorio also decides to add an area attack, ours would overwrite this.

What we need is a proper merging function, which data:extend could become. In theory relatively easy to do recursively. In practice it needs a second thought:

• Deleting elements would be hard, i'd propose a special value like

  Code: Select all

  action = "DELETE"

  or so to solve this problem.
• Sometimes, like with the action example, the content can either be a single table just containing the content like

  Code: Select all

  action = {type = "direct", ...}

  , or a table containing tables containing the content like

  Code: Select all

  action = {{type = "direct", ...}, {type = "area", ...}}

  . The only way i can think of for distinguishing this is by checking wether a table has only integer keys or not.

currently data:extend only overwrites the top-level content. But it could become more. I have written some example code:

Code: Select all

function isIntegerIndexed(t)
    for k, v in pairs(t) do
        if type(k) ~= "number" then
            do return false end
        elseif k ~= math.floor(k) then
            do return false end
        end
    end
    do return true end
end

--insert any t2 into integer-indexed t1
function insert(t1, t2)
    if type(t2) ~= "table" then
        do t1[#t1 + 1] = t2 end
    elseif t2.name then
        if t2.type then
            for _, v in ipairs(t1) do
                if (v.name == t2.name) and (v.type == t2.type) then
                    for key, value in pairs(t2) do
                        do v[k] = merge(v[k], value) end
                    end
                    do return v end
                end
            end
        else
            for _, v in ipairs(t1) do
                if v.name == t2.name then
                    for key, value in pairs(t2) do
                        do v[k] = merge(v[k], value) end
                    end
                    do return v end
                end
            end
        end
    elseif t2.type then
        for _, v in ipairs(t1) do
            if v.type == t2.type then
                for key, value in pairs(t2) do
                    do v[k] = merge(v[k], value) end
                end
                do return v end
            end
        end
    end
    do t1[#t1 + 1] = t2 end
    do return t1 end
end

--merge any t2 into any t1
function merge(t1, t2)
    if t2 == "DELETE" then
        --This for sure doesn't work yet as you'd need the key to delete a non-table value. But it's enough to get the point across.
        do t1 = nil end
    elseif type(t1) == "table" then
        if type(t2) == "table" then
            if isIntegerIndexed(t1) then
                if isIntegerIndexed(t2) then
                    for _, v in ipairs(t2) do
                        do insert(t1, v) end
                    end
                else
                    do insert(t1, t2) end
                end
            else
                if isIntegerIndexed(t2) then
                    --avoid cyclic pointers
                    do t1 = insert(t2, table.deepcopy(t1)) end
                else
                    --avoid cyclic pointers
                    do t1 = {table.deepcopy(t1), t2} end
                end
            end
        else
            do t1[#t1 + 1] = t2 end
        end
    elseif type(t2) == "table" then
        do error("Attempt to merge table "..serpent.block(t2).." into non-table "..serpent.block(t1)) end
    end
    do return t1 end
end

function data.extend(self, otherdata)
    
    if (type(otherdata) ~= "table") or ((#otherdata) == 0) then
        do error("Invalid prototype array "..serpent.block(otherdata, {maxlevel = 1})) end
    end

    for _, e in ipairs(otherdata) do

        if not e.type then
            do error("Missing type in the following prototype definition "..serpent.block(e)) end
        end

        if not e.name then
            do error("Missing name in the following prototype definition "..serpent.block(e)) end
        end

        local t = self.raw[e.type]
        if t == nil then
            do t = {} end
            do (self.raw)[e.type] = t end
        end
        do merge(t, e) end
    end
end

The code is untested and i know for sure that it won't work. But it should hopefully clarify what i want. It would solve a very general problem and simplify the development process by a lot, as we wouldn't need to copy data.raw elements and alter them anymore, and you would need to worry less about future compatibility.

[Lindor]

Turns out that the code i have written was absolutely terrible. But i came up with a solution:

Code: Select all

--very fast deepcopy of any non-recursive lua object without metatable support
local copy
do copy = function(obj)
    if type(obj) ~= "table" then
        do return obj end
    else
        local result = {}
        for k, v in pairs(obj) do
            do result[copy(k)] = copy(v) end
        end
        do return result end
    end
end end

--does this table have only integer keys?
function isIntegerIndexed(t)
    for k, v in pairs(t) do
        if type(k) ~= "number" then
            do return false end
        elseif k ~= math.floor(k) then
            do return false end
        end
    end
    do return true end
end

--insert any content table into an integer-indexed original table
function insert(original, content)
    if content.name ~= nil then
        if content.type ~= nil then
            for k, v in ipairs(original) do
                if (v.name == content.name) and (v.type == content.type) then
                    do original = update(original, k, content) end
                    do return original end
                end
            end
        else
            for k, v in ipairs(original) do
                if v.name == content.name then
                    do original = update(original, k, content) end
                    do return original end
                end
            end
        end
    elseif content.type ~= nil then
        for k, v in ipairs(original) do
            if v.type == content.type then
                do original = update(original, k, content) end
                do return original end
            end
        end
    end
    do original[#original + 1] = content end
    do return original end
end

--merge any content table into any original table
function merge(original, content)
    if isIntegerIndexed(original) then
        if isIntegerIndexed(content) then
            for _, v in ipairs(content) do
                do original = insert(original, v) end
            end
        else
            do original = insert(original, content) end
        end
    elseif isIntegerIndexed(content) then
        do original = insert(content, original) end
    else
        if original.name ~= nil then
            if original.type ~= nil then
                if (original.name == content.name) and (original.type == content.type) then
                    for k, v in pairs(content) do
                        do original = update(original, k, v) end
                    end
                else
                    do return {original, content} end
                end
            else
                if original.name == content.name then
                    for k, v in pairs(content) do
                        do original = update(original, k, v) end
                    end
                else
                    do return {original, content} end
                end
            end
        elseif original.type ~= nil then
            if (original.type == content.type) then
                for k, v in pairs(content) do
                    do original = update(original, k, v) end
                end
            else
                do return {original, content} end
            end
        else
            for k, v in pairs(content) do
                do original = update(original, k, v) end
            end
        end
    end
    do return original end
end

--update any content into the domain table at this key
function update(domain, key, content)
    if content == "DELETE" then
        do domain[key] = nil end
        do return domain end
    end
    local original = domain[key]
    if type(original) == "table" then
        if type(content) == "table" then
            do domain[key] = merge(original, content) end
        else
            do original[#original + 1] = content end
            do domain[key] = original end
        end
    elseif type(content) == "table" then
        do content[#content + 1] = original end
        do domain[key] = content end
    else
        do domain[key] = content end
    end
    do return domain end
end

--the main() method
function data.extend(self, otherdata)
    if (type(otherdata) ~= "table") or ((#otherdata) == 0) then
        do error("Invalid prototype array "..serpent.block(otherdata, {maxlevel = 1})) end
    end
    for _, v in ipairs(otherdata) do
        if not v.type then
            do error("Missing type in the following prototype definition "..serpent.block(v)) end
        end
        if not v.name then
            do error("Missing name in the following prototype definition "..serpent.block(v)) end
        end
        if (self.raw)[v.name] == nil then
            do (self.raw)[v.name] = v end
        else
            do self.raw = update(self.raw, v.name, v) end
        end
    end
end

Code has been tested, seems to work great

This could be the new data:extend(). Or it could be a completely new function like data:merge() or so. Either way it would be super useful if it existed in the scripts by default.

[Lindor]

Spotted a small mistake in the code above. Only applies if you're setting up a completely new prototype instead of altering an existing one. I'll still leave it here so you can learn from my mistake. Here's the fixed code:

Code: Select all

--set up function names to be local and allow for recursion
local copy
local isIntegerIndexed
local insert
local merge
local update

--fast non-recursive objet deepcopy function without metatable support
do copy = function(obj)
    if type(obj) ~= "table" then
        do return obj end
    else
        local result = {}
        for k, v in pairs(obj) do
            do result[copy(k)] = copy(v) end
        end
        do return result end
    end
end end

--does this table have only integer keys?
do isIntegerIndexed = function(t)
    for k, v in pairs(t) do
        if type(k) ~= "number" then
            do return false end
        elseif k ~= math.floor(k) then
            do return false end
        end
    end
    do return true end
end end

--insert any content into integer-indexed original
do insert = function(original, content)
    if type(content) == "table" then
        if content.name ~= nil then
            if content.type ~= nil then
                for k, v in ipairs(original) do
                    if (v.name == content.name) and (v.type == content.type) then
                        do original = update(original, k, content) end
                        do return original end
                    end
                end
            else
                for k, v in ipairs(original) do
                    if v.name == content.name then
                        do original = update(original, k, content) end
                        do return original end
                    end
                end
            end
        elseif content.type ~= nil then
            for k, v in ipairs(original) do
                if v.type == content.type then
                    do original = update(original, k, content) end
                    do return original end
                end
            end
        end
    end
    do original[#original + 1] = content end
    do return original end
end end

--merge any content table into any original table
do merge = function(original, content)
    if isIntegerIndexed(original) then
        if isIntegerIndexed(content) then
            for _, v in ipairs(content) do
                do original = insert(original, v) end
            end
        else
            do original = insert(original, content) end
        end
    elseif isIntegerIndexed(content) then
        do original = insert(content, original) end
    else
        if original.name ~= nil then
            if original.type ~= nil then
                if (original.name == content.name) and (original.type == content.type) then
                    for k, v in pairs(content) do
                        do original = update(original, k, v) end
                    end
                else
                    do return {original, content} end
                end
            else
                if original.name == content.name then
                    for k, v in pairs(content) do
                        do original = update(original, k, v) end
                    end
                else
                    do return {original, content} end
                end
            end
        elseif original.type ~= nil then
            if (original.type == content.type) then
                for k, v in pairs(content) do
                    do original = update(original, k, v) end
                end
            else
                do return {original, content} end
            end
        else
            for k, v in pairs(content) do
                do original = update(original, k, v) end
            end
        end
    end
    do return original end
end end

do update = function(domain, key, content)
    if content == "DELETE" then
        do domain[key] = nil end
        do return domain end
    end
    local original = domain[key]
    if original == nil then
        do domain[key] = content end
        do log(key) end
    elseif type(original) == "table" then
        if type(content) == "table" then
            do domain[key] = merge(original, content) end
        else
            do original[#original + 1] = content end
            do domain[key] = original end
        end
    elseif type(content) == "table" then
        do content[#content + 1] = original end
        do domain[key] = content end
    else
        do domain[key] = content end
    end
    do return domain end
end end

do data.extend= function(self, otherdata)
    if (type(otherdata) ~= "table") or ((#otherdata) == 0) then
        do error("Invalid prototype array "..serpent.block(otherdata, {maxlevel = 1})) end
    end
    for _, v in ipairs(otherdata) do
        if not v.type then
            do error("Missing type in the following prototype definition "..serpent.block(v)) end
        end
        if not v.name then
            do error("Missing name in the following prototype definition "..serpent.block(v)) end
        end
        local t = (self.raw)[v.type]
        if t == nil then
            do t = {} end
            do t[v.name] = v end
        else
            do t = update(t, v.name, v) end
        end
        do (self.raw)[v.type] = t end
    end
end end

[Lindor]

And there's one unsolvable problem:

Sometimes we want to insert a value into a table without a type or name identifier. But such tables cannot be uniquely identified other than by the key, but the key is arbitrary and can always change in the future.

Example:
data.raw:

Code: Select all

{
    type = "beam",
    name = "laser-beam",
    head = {
      filename = "__base__/graphics/entity/laser-turret/hr-laser-body.png",
      flags = beam_non_light_flags,
      line_length = 8,
      width = 64,
      height = 12,
      frame_count = 8,
      scale = 0.5,
      animation_speed = 0.5,
      blend_mode = laser_beam_blend_mode
    }
}

update command:

Code: Select all

do data:extend({{type = "beam", name = "laser-beam", head = {tint = {1, 0.5, 0.05}}}}) end

The head table is not recognizable by anything other by the "head" key, which is arbitrary and can change anytime, e.g. if head becomes a table of two tables. The function cannot recognize the table by "type" or "name" and therefore assumes we want to add a new table instead of inserting the tint data into the old table.
The issue is unresolvable.

Of course i could write a recognition function which checks how similar two tables are, but that would require the modder to "hardcode" the properties of the table into the code which would completely defeat the purpose of the merging function in the first place.

The merging function is still very useful. But it isn't as much of an allrounder as i thought.

I'd suggest for the future that every table in data.raw should be uniquely identifyable by the "type" and "name" properties. It would make life so much easier.

[Lindor]

Or, another idea for a good paradigm would be that every table without a "type" or "name" identifier always has a unique key. Like every graphics table with the key "head" cannot contain other tables. Therefore, if a table does not contain a "type" or "name" property, we always know that we want an insertion and not an appending.

EDIT: an example from base which violates this paradigm:

Code: Select all

    base_picture =
    {
      layers =
      {
        {
          filename = "__base__/graphics/entity/laser-turret/laser-turret-base.png",
          priority = "high",
          width = 70,
          height = 52,
          direction_count = 1,
          frame_count = 1,
          shift = util.by_pixel(0, 2),
          hr_version =
          {
            filename = "__base__/graphics/entity/laser-turret/hr-laser-turret-base.png",
            priority = "high",
            width = 138,
            height = 104,
            direction_count = 1,
            frame_count = 1,
            shift = util.by_pixel(-0.5, 2),
            scale = 0.5
          }
        },
        {
          filename = "__base__/graphics/entity/laser-turret/laser-turret-base-shadow.png",
          line_length = 1,
          width = 66,
          height = 42,
          draw_as_shadow = true,
          direction_count = 1,
          frame_count = 1,
          shift = util.by_pixel(6, 3),
          hr_version =
          {
            filename = "__base__/graphics/entity/laser-turret/hr-laser-turret-base-shadow.png",
            line_length = 1,
            width = 132,
            height = 82,
            draw_as_shadow = true,
            direction_count = 1,
            frame_count = 1,
            shift = util.by_pixel(6, 3),
            scale = 0.5
          }
        }
      }
    },

It's from the laser turret prototype. The layers key has a table of multiple non-uniquely-identifyable tables.
As long as things like these exist in the game, it creates the unresolvable issue.
Dear devs, please don't let this stand in the way of the vision of a proper merge function.

[Lindor]

Honestly, data.raw is pure chaos.

Even for the same entry in the same prototype, just over multiple game versions:

• Sometimes the entry is a table
• Sometimes it's a table of a table
• Sometimes it's a table of multiple non-uniquely-identifyable tables

For a game like factorio that is generally thought to be very "clean-coded", this is just bad standards.

It seems like the "copy-from-basegame-and-just-change-what-you-want"-paradigm has a built-in forwards-incompatibility. If for some reason in future the "table of a table" becomes a single table, it breaks all mods which used that paradigm on that entry.

There's currently no way to write a proper merge function (which would make all mods forwards-compatible) without finally setting some rules for data.raw's structure.

I resolved the "table"-or-"table-of-table"-dualism. But the "table-of-non-uniquely-identifyable-tables" thing is proven to be unresolvable.

A merge function would bring so much value to the game's modding support. I just wish you were able to see that.

[Rseding91]

Just FYI: there is no standard for how any given prototype is defined between versions. Only a specific version defines how that specific version functions. That is not likely to ever change. Each game version has its own formats and will likely always function that way.

If we break the format between versions that's called a "Breaking change" and will require mods be updated to handle it. We try not to do that because it's annoying for modders but sometimes with major updates it still happens.

Once we release a new stable game version all older versions are obsolete and irrelevant.

[Lindor]

Okay.
Well, since the modders are required to update their mods for any new game version, complete forwards-compatibility will never happen anyway.

It just makes me sad because i see massive wasted potential here.

If there was just one simple rule, namely that tables inside tables must be distinguishable either by "name", "type" or both, the merging function would become extremely powerful.