multiply_energy_value

[robot256]

I used opteralib.multiply_energy_value() in a couple places and noticed it's not in flib yet. Is adding it in the plan? I saw that Optera copied it into data.lua in order to use flib.get_energy_value(), so it seems like it would be a good addition.

[0ptera]

I didn't copy it over since it felt half baked to only have multiplication and bloated to have one function per arithmetic operation.

[robot256]

I see that get_energy_value converts to a number of base units using the SI prefix. I also understand that you don't need a function to go the other way, because the game will accept 350000000W just the same as 350MW.

It's still cumbersome because having a two-output function means you need at least 2 lines of Lua to recompose the string after doing the math. For example:

local value, unit = flib.get_energy_value(s)
local new_s = (value * 2) .. unit

To do it one one line, you have to ignore the second output:

local new_s = (flib.get_energy_value(s) * 2) .. string.sub(s, -1)

Neither of these is particularly compact or easy to read. Maybe what I'm really asking for is two separate functions, "get_energy_value" and "get_energy_unit":

local new_s = (flib.get_energy_value(s) * 2) .. flib.get_energy_unit(s)

Then you could use the library to make your code easier to read, and do whatever math you want, without bloating the library for each operation.

[0ptera]

I fail to see the point of having unreadable and unmaintainable one line commands. Even 2 Lines are only possible if you are 100% sure get_energy_value will always receive an Si prefix or unit it can handle.

If you do that check beforehand you already have rewritten get_energy_value. To handle get_energy_value error output you'll need something like this:

local function multiply_energy_value(energy_string, factor)
  if type(energy_string) == "string" then
    local value, unit = flib.get_energy_value(energy_string)
    if value then
      value = value * factor
      return value..unit
    end
  end
  return ""
end

With this level of error handling I can see how adding the function into the library would be beneficial.

[robot256]

Yes exactly. It seemed that get_energy_value was essentially useless by itself because of the wrapping (both syntactical and error checking) that is needed.

Another question: Precisely what kind of error handling is needed here? I have seen first-hand that Factorio can exhibit silent undefined behavior when improper strings are given as power values, so there are cases where we would want to force an error if the correct suffix is not found. Is returning "" enough to make the game crash so that the error can be located?

[0ptera]

The intended scope of get_energy_value is to split an Si string into number and unit, hardly useless.

[robot256]

Maybe this is an example of how the goals of only containing "commonly used utilities" and "eliminate the need for every modder to have their own lib" are not compatible. You can put the common, well-defined stuff here and that is great, but there's still going to be a lot of modder-specific code that is too general to keep in just one mod, but too specific to put in a library used by many people. Subsuming every modder's personal library inevitably leads to bloat.

[0ptera]

Following that rabbit hole we end up with libraries subscribing to on_entity_created for mods too lazy to type local entity = event.entity or event.created_entity.

In the end libraries are just a box of commonly used tools. Mods still have to pick the right tool for the job and use it in a way to not hit themselves on the head with a hammer. Don't expecting to require toolbox and it magically builds your house.

[0ptera]

Error handling always has to be done by the mod using a library function. Otherwise we get bug reports instead mods calling library function. If I add multiply_energy_value back in you'll still need to write error handling for when it returns an empty string or nil.

multiply_energy_value is 90% handling error from get_energy_value and passing it on. I'll leave it to the others to decide if it's worth having a function just passing errors along, or if mods using flib should directly handle errors.

My earlier example seems to give the wrong impression. Using get_energy_value directly, only writing values back into data.raw on valid value is trivial:

for _, roboport in pairs(data.raw["roboport"]) do
  local value, unit = get_energy_value(roboport.energy_source.input_flow_limit)
  if value then
    roboport.energy_source.input_flow_limit = (value * 2)..unit
  end
end

[robot256]

After sleeping on it, I can accept the code you just posted as the replacement for multiply_energy_values--I only use it in 2 places after all. Yesterday I was just annoyed that I had to port to a new library with seemingly less functionality than the old one. Thanks for the explanation.

There is no problem with errors being blamed on the library as long as the library function returns without crashing in all cases. Any invalid string the host mod assigns to a prototype field will cause an error that the game only blames on the host mod, and the same should be true in the control script. I prefer this behavior in the data phase because then I can investigate why the source value was not a formatted power value like I expected.

I'm really looking forward to Nexela's math library from what I saw in another branch.