afforess / factorio-stdlib Goto Github PK

Hard crash with Factorio using event system. Not super well versed in Lua/stdlib so I am posting this here for your input in case it is something preventable code wise.

I made a boo-boo when changing some stuff over to use STDLIB event system

Event.register(Event.core_events.init, myfunction()) --Hard crash when it executes, note the () on the end of it.

Calling it correctly does work as intended :)
Event.register(Event.core_events.init, myfunction)

In order to help track down user desync problems being able to force a CRC check after every handler is called might be a big help

Example ->
Event.force_crc_check = true

When iterating handles before calling them
game.print("forcing CRC check handler)
game.force_crc()

Doing something like this can help the user narrow down desync problems in their code.

Currently the trains library only watches for events on the vanilla diesel locomotive, but it should be possible to extend this to check for any type of locomotive.

An iterator to visit each tile inside of an Area is needed. Area.iterate?

for x,y in Area.iterate({{0,0}, {10,10}}) do
   ...
end

When the files are present in multiplayer custom scenario (even when there is no require in control.lua), players can't join.

They can connect and then they get dropped with the message:
factorio cannot load downloaded map create_directory(p) operation not permitted

Hi Afforess and contributors,

I really like the approach of Factorio-Stdlib to offering a well-tested, developer friendly library to handle some of the common stuff needed in Factorio mods. One area I've identified that could be made more friendly through such a library is tracking and working with trains.

Before I made any further progress with a pull request, I wanted to ask you all whether this is something you'd accept as a pull request? Personally, I think it'd make other mod authors lives easier, but equally I can see it's bit more involved than the rest of the library,

To ease development, I've already had a conversation with @Choumiko (and go-ahead) about integrating the code they wrote here, since it's been proven to work in that mod.

The most difficult part I can see would be making sure that a scenario like the below works. Right now, having done only a little testing, it seems that passing a LuaTrain object directly to Entity.set_data will fail because LuaTrain doesn't have a name property. The most obvious work around there is not to return a LuaTrain object directly, but wrap it in something with a name property that has a value unique to that train. (A problem @Choumiko has solved using the unit_number of the train's main locomotive)

require("stdlib/trains/trains")
require("stdlib/entity/entity")

for i, t in pairs(Trains.get_all_trains_on_surface(game.surfaces[1])) do
  Entity.set_data(t, { index = i, msg = "I was train " .. i .. " on the surface})
end

Is handling trains in this library something you feel you'd be interested in doing? Or do you feel it would be better placed as its own library/mod?

Regards,
Andy

Trains now have unique IDs on_train_created (and other train events) stdlib/trains should be changed yet again to use these IDs

local new_event = {
                    tick = event.tick,
                    name = event.name,
                    _handler = handler,
                    match = match_str,
                    element = gui_element,
                    state = gui_element_state,
                    text = gui_element_text,
                    player_index = event.player_index ,
                    _event = event
                }
                local success, err = pcall(handler, new_event)

Doesn't account for the new stuff provided by GUI's, My first thought was to add the missing values. My second though was just provide a metatable pointing to new_event._event But then I got to thinking Why not just modifiy event and send as is?

---remove new_event
event._handler = handler
event.match = match_str
event.etc = etc
local success, err = pcall(handler, event)

This issue is raised for the purpose of discussing how the Trains module should work in conjunction with the Entity module. I'll summarise the problem below and pose a potential solution. Sorry it's such a long post, I just wanted to document all the things I've already looked at.

As it stands, the Entity module is designed to work with LuaEntity objects (or instances of any class that extends it). More generally though, it will work with any table with name and valid keys. This absolutely fine in the vast majority of scenarios, but trains are a special case.

Trains are instances of LuaTrain which doesn't extend LuaEntity, nor does it contain the right fields to be treated as such. This means we can't directly save data about a train; rather modders would save data against the lead locomotive. Whilst this would work, and the on_train_id_changed event would allow them to perform the necessary book keeping, I think it's unintuitive, tedious and likely prone to error.

Instead, we should enable modders to directly save data about trains.

An obvious solution .. that doesn't quite work

The quickest and most obvious sounding solution is provide a duck type for LuaEntity that enables LuaTrain to be used with the Entity module

-- stdlib/trains/trains.lua
function Trains.to_entity(train)
  return {
    name = "train-"..Trains.get_train_id(train)
    valid = train.valid
  }
end

-- control.lua
Entity.set_data(Trains.to_entity(a_train), data)

However this doesn't work at first blush, because the Entity module compares the entity given as the first argument with the one in the global data by reference.

-- stdlib/entity/entity.lua#L56
if entity_data.entity == entity then
  return entity_data.data
end    

This appears to be fine when handling LuaEntity references; I guess this is because the game can properly (de)serialize those and always returns the same references from global data and surface queries. but I don't know for sure.

However, the Trains.to_entity method above always returns a new table, so the references will never match. To get around that, we could modify Trains._registry and other Trains internals to keep track of the faux-entities and have to_entity return that reference.

for _, trainInfo in pairs(all_trains) do
    registry[tonumber(trainInfo.id)] = {
      train = trainInfo.train,
      entity = {
        name = "train-"..trainInfo.id
        valid = trainInfo.train.valid
      }
    }
end

However that approach falls down in the following scenario:

1. I have locomotive 1000, locomotive 2000 and wagon 1
2. I connect locomotive 1000 and locomotive 2000 to either end of wagon 1 to form an L-C-L train called train-1000
3. I store data about train-1000
4. I disconnect locomotive 1000 from train-1000, and the train id changes to train-2000.
5. I reconnect locomotive 1000 to train-1000, thus re-creating train-1000. In the mean time, train-1000 was dropped from the registry and the entity reference lost. Thus, the train-1000 data is inaccessible.

To this end, I don't think comparing by reference will ever suit the Trains module

Field-wise equality

In reality, as far as the Trains module is concerned, two tables with the same name value are equal and referring to the same train. To that end, I would propose making a slight change to the Entity module to have it call an equals method on the input entity if there is such a method and the entity references don't match.

-- for brevity's sake, checking the function exists is omitted
if ((entity_data.entity == entity) or (entity.equals(entity_data.entity)) then
  return entity_data.data
end

This would preserve the behaviour of the Entity module, but crucially allow a simple integration with the Trains module, eg

function Trains.to_entity(train)
  local self = {
    name = "train-"..Trains.get_train_id(train),
    valid = train.valid,        
  }

  -- perhaps creating a new function for each call isn't
  -- sensible, bit here it is for clarity
  self.equals = function(ent)
    return ent.name == self.name
  end

  return self
end

Entity.set_data(Trains.to_entity(train), { working = true })
data = Entity.get_data(Trains.to_entity(train))
-- { working = true }

Of course, I think no matter what solution we choose, there will need to be some internal book keeping to shuffle stored data about when a train's id changes.

Same reason as in Trains.lua = _last_change will be different for players connecting later

it('.dispatch should abort if a handler returns true', function()

This actually bit me so many times before I realized it.

Is there any special reason this is needed?

If you guys would include it in the repo, I'd totally make a bunch of generic placeholder images. They'd basically be A-Z with borders around them.

Is this in scope of Factorio-Stdlib?

The error is being thrown on logger.lua:37

This happens under factorio 0.13 on an old savegame.

Maybe I initialized the logger incorrectly (in on_load)?

Here: https://github.com/Afforess/Factorio-Stdlib/wiki#avoiding-iteration-and-general-stdlib-tools

Which is an excellent suggestion for low-weight code, but have you compared them in hot paths?

General consensus in WoW modding is that it's worth avoiding as much overhead as possible in such areas, even things like for i/# vs ipairs. Maybe worth adding a note there, if the same is true here?

A force append option for those times when you don't want your log starting all over again :)

`local Logger = {mod_name = mod_name, log_name = log_name, debug_mode = debug_mode, buffer = {}, last_written = 0}

--- Logger options
Logger.options = {
    log_ticks = options.log_ticks or false, -- whether to add the ticks in the timestamp, default false
    file_extension = options.file_extension or 'log', -- extension of the file, default: log
    force_append = options.force_append or false, -- append the file on first write, default: false
}
Logger.file_name = 'logs/' .. Logger.mod_name .. '/' .. Logger.log_name .. '.' .. Logger.options.file_extension
Logger.ever_written = Logger.options.force_append`

Hi guys,

currently I'm doing stuff like this:

function addTechnologyUnlocksRecipe(technologyName, recipeName)
    data.raw["recipe"][recipeName].enabled = false
    if data.raw["technology"][technologyName].effects == nil then
        data.raw["technology"][technologyName].effects = {}
    end
    table.insert(data.raw["technology"][technologyName].effects,
        { type = "unlock-recipe", recipe = recipeName })
end

It would be nice if we'd have a Util class, that allows us to:

• add recipes to technologies
• add technologies
• enable/disable recipes/technologies etc

The Entity module needs a function that will search the entire loaded game for all instances of a type of entity.

adjust is too ambiguous and, without reading docs or code, gives no indication of what is being adjusted

The GUI is a fertile ground for stdlib. Some broad suggestions include:

• GUI Templates (create look-alike elements from a template)
• Give every element callbacks (the single event the game provides is unwieldy)
• Widget Groups (Label + Button)

Forum Suggestions: https://forums.factorio.com/viewtopic.php?f=96&t=23181&p=158743#p158740

Significant effort needs to be invested in making icon styles much easier to use, and in general making the gui api less opaque and difficult to learn or maintain.

Think I found an bug in the string.split function. If you use a multi-character separator but all the characters of the separator are the same it is treated as a single-character separator.

local a = string.split("Test the bees knees", "ee")
--Expected {"Test the b", "s kn", "s"}
--Got {"T", "st th", " b", "s kn", "s"}

[edit]
Any multi-character separator that start and/or ends with white space behave badly too. :/

Spent way longer than I intended trying to make string.gsub place nice in string.split. I'm just gonna push the php-like explode function from http://lua-users.org/wiki/SplitJoin I've been using for a few weeks now.

I need to determine which entity in a list is closest to a fixed position - should I use distance_squared or manhattan_distance for fastest performance?

Would be good if docs specified the speed benefits of all three distance methods

Hi Afforess and contributors,

I just began to set up my environment in order to send you a pull request down the line, and my first port of call was to run the tests. Unfortunately, it looks like there are 3 broken.

Error spec\area\chunk_spec.lua @ 5
Chunk Spec should give the correct chunk coordinates for a position
.\stdlib/area/chunk.lua:20: attempt to index global 'bit32' (a nil value)

Error spec\area\tile_spec.lua @ 34
Tile Spec should ensure indexes are stable and deterministic
.\stdlib/area/tile.lua:120: attempt to index global 'bit32' (a nil value)

Error spec\area\tile_spec.lua @ 49
Tile Spec should verify getting and setting data
.\stdlib/area/chunk.lua:20: attempt to index global 'bit32' (a nil value)

I'm running Windows 10, and I've installed the busted and bit32 packages (and their dependencies) through LuaRocks, having allowed it to install a version of Lua for me.

Are these test failures a result of something missing in my environment, or are they genuine test failures? (I suspect the former, but I thought it better to ask)

Regards,
Andy

Suggestion from forums: https://forums.factorio.com/viewtopic.php?f=96&t=23181&p=158743#p158352

A module to allow easier data.raw manipulation:

 -- creates a new pole named my-op-pole based on big pole, changing mining result name as well
local op_pole = Data.copy("electric-pole", "big-electric-pole", "my-op-pole")
op_pole.wire_distance = 9001

local recipe = Data.copy("recipe", "big.electric-pole", "my-op-pole")
local item = Data.copy("item", "big.electric-pole", "my-op-pole")
-- could perform some checks, e.g. duplicate name
Data.extend{op_pole, recipe, item}
Data.add_unlock_to_tech("some-tech", recipe) -- or Data.add_unlock_to_tech("some-tech", "my-op-pole")

-- remove all 'references' to the item from data.raw
Data.remove("item", "name-to-remove")
--replace all 'references' to itemA with itemB
Data.replace("item", "itemA", "itemB")

Last 2 seem like edge cases

Right now Inventory.copy_inventory uses inventory.insert, which is very limited. It won't preserve the positions of items, the contents of blueprints, or the inventories of armor.

Instead, you should do something like this:

  for i=1,#src do
    dest[i].set_stack(src[i])
  end

Just posting these here in case you'd not seen them - might be useful source of optimised functions for common tasks that you could include (as needed) in your lib:

• https://github.com/luapower/glue
• https://github.com/rxi/lume - optimised for game dev

Needs expansion to include the two new events:
on_gui_elem_changed
on_gui_selection_state_changed

Additionally stdlib/gui/gui would make more sense as stdlib/event/gui. What are your thoughts on this?

An alternate method of registering events is needed. It should allow multiple event handlers to be registered with one type of event, and allow the event to cascade through them, or be aborted. Example:

Event.register(defines.events.on_tick, function(event)
   ...
end).register(function(event)
 ...
end

or

Event.register(defines.events.on_tick, function(event)
   print("Hello World")
end)

Event.register(defines.events.on_tick, function(event)
  return true -- return true should cancel on_tick for any other event handlers for this event, so 'Goodbye!' should never be printed
end)

Event.register(defines.events.on_tick, function(event)
   print("Goodbye!")
end)

Currentl you can't use thos repo as submodule because the lua scripts would not be found, as the "requires" are kind of hardcoded.

would it be possible to move the stuff in stdlib into the root?

Due to Game.print_all checking player.connected any mod that uses stdlib in their on_init or on_configuration_changed handlers will show player.connected as always false in single player only.

See https://forums.factorio.com/33208 for the full details.

Hi,

My event handler function crashes unexpected and without any error, exception or warning.

Event.registert(defines.events.on_chunk_generated, function(event)
  global.logger.log("on_chunk_generated start")
  local a = game.player.surface
  global.logger.log("on_chunk_generated end")
end)

The error I get when I use the standard event registration:

Error while running the event handler: mojo-exploration/control.lua:61: attempt to index field 'player' (a nil value)

This error should also be shown on screen when using your event mechanism.
I debugged for like an hour before I came to try to change back to the original event registration.

As a follow up to https://forums.factorio.com/viewtopic.php?f=25&t=32836&p=276626#p276609

Maybe add an additional option to use log() when inside the data stage (data:extend is available) or inside control.lua (when interface is available) ?

I've been using this function a lot in my mods, maybe it would be useful for StdLib with some extra tweaks?

local function petrify( entity, mode )
  mode = mode == false and true or false
  entity.active    = mode
  entity.operable  = mode
  entity.rotatable = mode
end

Usage:

petrify( someEntity ) -- petrify it
petrify( someEntity, false ) -- reanimate it

I would like to see a function/algorithm added, that can, given a single ore entity from an ore patch, return all (connected) ore entities of an ore patch. This could be extended to place a bounding box around the ore patch. I will code a first version and make a pull request.

First: And most important is the potential for desyncs
Event.core_events.init, function() Trains._registry = create_train_registry() end

Trains._registry is not saved into the global table leading to desyncs.
Player 1 creates game, init is called and scans trains now the Trains variable contains extra data
Player 2 joins and since this is MP init is not triggered for player 2 and he does not have the extra data
script does something with trains. 2 different things happen. game goes kaboom.

Haven't tested yet but replacing Trains._register with global._trains seems like the easiest approach

Second: function local function create_train_registry() Only scans surface 1
Should be changed to loop through all surfaces in case multiple surface trains are present.

In train init will not be called if if stdlib/trains is added after the mod has been installed
Additionally even init is not going to be reliable in instances where mod authors do this

require("stdlib/event")
local trains = require("train")
Event.register(Event.core_events.init, function() global = {})

During init trains will be inited first, followed by the top level init which overwrites the global

Doing global = {} is bad practice though.

As for Time changing global. at the top is not recommended but will work. However issues will still happen if mod is updated mid game to use Time the global will be discarded in on_load for the version saved in the map which won't have _surface_time. leading to errors

Time will be addressed in the upcoming pull request.

Looking through trains It needs an overhaul to use surface/force.get_trains so I am not going to mess with ti too much

Currently, there's no events fired if a mod destroys or creates an entity - and the devs have stated that they will not implement them. Forum discussion

As such, mods are going to need some consistent way to deal with this issue, and stdlib seems to be the ideal home for it.

Rough draft...

-- both of these will need a generated event id
-- and a remote interface to get hold of those ids
local create_event
local destroy_event

function Entity.destroy_with_event( entity )
  game.raise_event( destroy_event, entity )
  entity.Destroy() -- this might fail, eg. rail with train on top cannot be destroyed
end

function Surface.create_entity_with_event( surface, tbl )
   local entity =  surface.create_entity( tbl )
   if entity then game.raise_event( create_event, entity ) end
end

I am halfway through a code cleanup on stdlib.

The biggest part of the cleanup is making sure everything lints cleanly in a very strict linting environment. This includes use of locals for performance and standard reasons.

Right now I have to ignore most of stdlib and that just doesn't feel right.
https://gist.github.com/Nexela/05a8fa5164ae107f599c2295935ff906

So far everything I have done is mostly backwards the only thing that changes is the way some modules are added.

Invalid: require("stdlib/area/area")
required: local Area = require("stdlib/area/area")

Other examples that would work but are not recomended:

• Area = require("stdlib/area/area") --Makes Area global
• local MyArea = require("stdlib/area/area") -- Works but no reason to do it this way
• MyArea = require("stdlib/area/area") -- Globals are bad mkay, also no reason to change identifier

The next on my list Event/Gui those will require more thinking on though as simply making Event local will break things.

If you are interested in a PR on this when it is done/in progress let me know.

Hi all,

Currently I'm developing several mods that depend on each other. It's tedious to include stdlib in every mod and keeping it up-to-date. It would be much easier if it would be wrapped in a mod, that could just be included in the info.json and provides all functionality like the bob'S library mod does.

What I'm currently doing is to setup each class I need in the global.* namespace. As I'm still new to lua script's (and factorio's) pitfalls I'm not quite there.

This would help me a lot :-)

Both Events systems needs some generic .valid checks when called to be consistent with factorio events. events are not raised if the something is invalid

Hi,

currently I'm working on an improvement to my terraform functionality. Previously when I used the shovel on land it just created a water hole and vice versa.

I'm not very satisfied with this behaviour (where does the water come from? ;-))
So I created a "moat" tile (any better name ideas?).

My idea is to flood the all connected moat tiles with water, if on is adjacent to a water tile.
I figure that I could use a lot of loops to go over all moat tiles but there might be a better way?

The perfect solution would be a new functionality in your library ;-) maybe other people need this too.

What are the thoughts on a Config module? I have one I've been using that I can convert fairly easy for stdlib inclusion. I'll uncouple it from my use and convert to stdlib then post it tomorrow night if such a module is desired. It will come without tests or with poor test coverage at first.

Basically, it would look/work like this in stdlib.

 --Makes sure global._config exists as a table. Called at the start of every other Config functions for insurance.
Config._setup()

-- Translates to global._config.some.path.to.variable or safeDefault.
-- if desired a table can be grabbed with Config.get("some/path") = {to={variable="example"}}
Config.get("some/path/to/variable", safeDefault)

-- Translates to global._config.some.path.to.variable = data.
-- data can be anything. Even more tables. Which can then be seen with get(). such as get("some/path/to/variable/with/more/table")
Config.set("some/path/to/variable")

--Deletes "some/path/to/variable" (sets to nil)
Config.delete("some/path/to/variable")

Perhaps adding a user "facing" Config.setup({options_table}) to allow changing the separator to any desired string. Store that in say a global._config_options table.

Could also be in Position.add but don't have the time to look so making an issue as a reminder

This should return about -0.75

I just had the problem that a mod tried to access the player inventory on init. This silently failed, letting the on init event handler remain non-functional.

You could add an error(err) to the success condition so the error reason shows up in the gui.

Currently there is no way to register keybind events with stdlib/event/event

These events are passed as literal strings which are internally turned into a script id number. The only way I see to get the id number is from the raised event however the raised event also contains the keybind name. I can pester Klonan about this some more but I need a compelling argument for it.

Loading the trains module in my control.lua causes the following error.

**__ElephantTrains__/control.lua:8:
  __ElephantTrains__/stdlib/surface.lua:22: 
  attempt to index global 'game' (a nil value)**

I have tracked this down to line 235 of Trains.lua (Trains._registry = create_train_registry())
since this line runs at the top level, game is indeed not in scope, so the eventual call to Surface.find_all_entities and so on fails.

Hi!

I just posted this on the Factorio forums: https://forums.factorio.com/viewtopic.php?f=34&t=31980
I'd gladly contribute and merge under some conditions :)
BTW I know it's bad, i coded in a hurry

Regards.

This is very small issue, if you could even call it that, but there's no mention to what lua file each module corresponds to in the documentation. This is more relevant on submodules, like Position (in stdlib/area) or Time (stdlib/events).

As in, right now on the header of the Area module you have:

when, for lua beginners or people like me that don't want to go check the directory tree every time they want to import something, it could instead be like this:

Doesn't have to have this formatting, or even be there, maybe a more fitting place for this would be the index page. Thank you for your time! 😄

I just realised i have a few small functions i use across mods that are not necessarily restricted to something Factorio specific, e.g.

function trim(s)
  return (s:gsub("^%s*(.-)%s*$", "%1"))
end
function round(num, idp)
  local mult = 10^(idp or 0)
  return math.floor(num * mult + 0.5) / mult
end
function isEmpty(tableA)
  if tableA ~= nil then
    return next(tableA) == nil
  end
  return true
end

Would a util module make sense? It could get out of hand rather quick i guess.
Right now i'm thinking something like

util.string.trim(s)
util.string.starts_with(s, needle)
util.string.ends_with(s, needle)

util.math.round(num, idp)

util.table.is_empty(table_)

should work for a start. If it get's too much it can be moved to a own module.