windfisch / factorio-bot Goto Github PK

This boils down to the Travelling Purchaser Problem. At least a very basic heuristic for this would still be better than the current algorithm, which just enumerates all chests around (0,0) or the player's position.

When a task has the goals "Put an entity X, and fuel X with coal", the bot will crash:
In _calculate_actions() of the InventoryPredicate, the X entity isn't found in actual_entities (and thus no inventory is known).

Should plan a facility from an input list and a desired output list. (e.g. "give me 4/sec green circuits using only iron and copper plates" or "give me 13/sec iron plate using iron ore")

When a craft is started, this does not remove the ingredients from the player's inventory (bug!). When the craft is finished, however, the products are correctly added to the inventory.

If a task is eventually runnable, it is scheduled with its crafting ETA. However currently, the first scheduled task will be executed, regardless of whether it's actually runnable yet.

The WalkTo-Action may already be executed, but the task itself must wait until it became runnable.

Use strongly typed typedefs as described here https://foonathan.net/blog/2016/10/19/strong-typedefs.html

e.g. for:

• ids
• x and y coordinates
• amounts

Use array wrappers with an operator[] that does not accept int, but x_coord_t or y_coord_t, respectively, to avoid bugs related to unintended coordinate swapping.

this is not yet fair

If a CraftingAction is aborted, the game refunds the items to the player. These shall be claimed to the original owner, and not be treated as spurious item update.

WorldMap currently stores its Chunks column-wise in the memory. However, data is accessed usually row-wise (especially in the GUI drawing functions / update_imgbuf).
-> WorldMap should store data row-wise.

Experiments with GCC6, -O2 for GUI and -O1 for the rest have shown that the drawing speed is doubled.

When an iron axe is being crafted in the factorio game window, to the bot it appears that an iron axe is added to the inventory, but no iron is removed.

When adding lots of facility upgrade tasks, and then submitting a higher-priority coal refiller in between, then the coal refiller will try to refill coal for all planned entities. But it should only refill the actual entities.

Possible fix: register a callback upon task completion to keep track of the actual entities on the map.

Now that most functionality is present, we should actually use it :)

The last item the resource collector task mines isn't added to the inventory immediately, but with one frame delay. But the scheduler recalculation occurs immediately, causing the item collector to be repeated.

Instead, actions should only be marked as completed once their effects came into effect.

Implement full inventory updates, and augment them by "pending-but-not-yet-confirmed" actions.

see #16 (comment)

These functions are supposed to transfer items from and to the player's inventory. Instead, they transfer them from and to /dev/null, not updating the player at all.

bot: scheduler.cpp:46: void sched::Task::update_actions_from_goals(FactorioGame *, int): Assertion `first_pos.has_value()' failed.

Program received signal SIGABRT, Aborted.

Fulfilled tasks should be removed from the scheduler completely.

WalkTo actions (or anything involving route planning, actually) currently plan their routes on start(), introducing a long delay if the distance is large.

Instead, these actions should be able to plan ahead, changing things as follows:

1. WalkTo actions store their route instead of generating it on-the-fly.
2. Their route is initially pre-populated with a naive straight line. It is intended to replaced very soon by the first update_route(Pos start) call.
3. They offer an update_route() method that's intended to be executed from a different thread.
4. update_route() will atomically / mutex-locked update the previous route, so at any point in time, there's a route that's more-or-less-valid.
5. If at the begin of update_route()'s execution it is discovered that the action is already running, then route calculation takes the current player's position as its starting point.
6. If at the end of update_route()'s execution it is discovered that the action is already running, the player is "pulled onto the route" with another short pathfinding call.

The pre-planning thread will iterate over all WalkTo's in tasks and over the implicit WalkTo's in between the tasks, and pre-plan them up to a limited time/waylength in the future.
Also, upon start() of a WalkTo action, it is replanned again, to account for last-minute changes.

Coal drills' fuel inventories are special: They are TAKE, not PUT

the writeout_pictures() function in the lua mod takes too much time while reconstructing data.raw. This results in the client being unable to connect to the game, because it is too slow.

Workaround: click "reconnect".

For now, already-known resource tiles simply get ignored. If the type of an already-known entity changes, an exception is raised.

The bot must ensure to be within a certain range to place an entity, but also to be not within a more narrow range.

Otherwise it fails to place an entity because it is in the way.

When a CraftingAction has finished, it is deleted immediately in main.cpp. One frame later, the Inventory update arrives and can't find the owner. Therefore, it cannot add the appropriate claim, the crafted item becomes free-for-all.

Possible fix: let the Task store a pointer to these CraftingActions. Or maintain a actionid -> owner_id mapping, without the weak_ptr<PrimitiveAction> in between

whenever the fuel is empty on some machines, a high-priority refill job should be scheduled

just take that coal from the neighboring rigs

When a high prio task is scheduled with an ETA large enough to do some work, but too short to finish any work, then currently the bot will just idle.

It would be better to pick any runnable lower-prio task (that's based on goals, not actions) and execute that to fill the gap. Or to run a resource collector.

Ideally, we pick that task that minimizes the walking durations.

(No crafting can be done, because that high-prio task will occupy the crafting queue until it turns runnable)

Tasks should depend on other tasks. The scheduler should respect this.

When running af187e6 and adding an iron rig, then a coal rig, refilling coal and adding another iron rig, the bot fails to keep track of its inventory.
It thought that it had 14 iron-plate, but in fact had 0.

The coal refill task's first way seems to always go over the (0,0) coordinate. Dunno why

FactorioIO should be a pure callback-driven game driver.

The "basic" logic like Inventory and position tracking, map building, resource patch management and action management (including path planning) should be moved up a layer into one or more separate modules.
E.g. actions should auto-extrapolate the inventory, lag hiding should be performed here.

The scheduler resides another layer up, using actions for crafting, walking and adding "item claims".
Problem: in order to perform item claims, either the basic logic + lua mod need to know about these attributions -> they depend on "Task", or the scheduler needs to maintain these claims separately from the actual inventory -> difficult to avoid race conditions: the inventory update and the claim must be done in the same tick, before (or both after) executing the strategy tick()s

The strategy managers read from the world map and submit tasks to the scheduler

Any Task must carry a list of basic items that are permitted to be collected. (Later the scheduler should dynamically decide this, but not yet).

Task::auto_craft_from() must be called from within the Scheduler, after updating the item allocation.

Factorio 0.16 has some images that are described as something.png in the data.raw structure, but the actual files are split in something-1.png, something-2.png etc.

The current behaviour is to ignore these files completely. It might be good to actually handle this.

when a tasks wants to craft 100x wood from raw wood, and has only 4x raw wood, then it thinks that all 100 crafts will be able to succeed.

Only occurs sometimes, not really reproducible. Addresses did not resolve to source code lines.

Scheduler::recalculate()
=================================================================
==2832==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdb4e53b58 at pc 0x563ae0c41ea5 bp 0x7ffdb4e52ff0 sp 0x7ffdb4e52fe8
READ of size 8 at 0x7ffdb4e53b58 thread T0
    #0 0x563ae0c41ea4  (/home/flo/factorio-bot/bot+0x86fea4)
    #1 0x563ae0c487d1  (/home/flo/factorio-bot/bot+0x8767d1)
    #2 0x563ae0c3b598  (/home/flo/factorio-bot/bot+0x869598)
    #3 0x563ae0c36c56  (/home/flo/factorio-bot/bot+0x864c56)
    #4 0x563ae0c3611d  (/home/flo/factorio-bot/bot+0x86411d)
    #5 0x563ae0dd380b  (/home/flo/factorio-bot/bot+0xa0180b)
    #6 0x563ae0dad69c  (/home/flo/factorio-bot/bot+0x9db69c)
    #7 0x563ae0d99d99  (/home/flo/factorio-bot/bot+0x9c7d99)
    #8 0x563ae0d97d4a  (/home/flo/factorio-bot/bot+0x9c5d4a)
    #9 0x563ae0f172df  (/home/flo/factorio-bot/bot+0xb452df)
    #10 0x7f440e62f222  (/usr/lib/libc.so.6+0x24222)
    #11 0x563ae09facbd  (/home/flo/factorio-bot/bot+0x628cbd)

Address 0x7ffdb4e53b58 is located in stack of thread T0 at offset 888 in frame
    #0 0x563ae0c3626f  (/home/flo/factorio-bot/bot+0x86426f)

  This frame has 38 object(s):
    [32, 40) 'point.i'
    [64, 72) 'ref.tmp8.i'
    [96, 480) 'log' (line 88)
    [544, 576) 'ref.tmp' (line 88)
    [608, 609) 'ref.tmp1' (line 88)
    [624, 640) 'view_area' (line 94)
    [656, 760) 'view' (line 97)
    [800, 808) 'ref.tmp43' (line 97)
    [832, 872) 'openlist' (line 102) <== Memory access at offset 888 overflows this variable
    [912, 913) 'ref.tmp56' (line 102) <== Memory access at offset 888 underflows this variable
    [928, 984) 'needs_cleanup' (line 104)
    [1024, 1040) 'ref.tmp69' (line 106)
    [1056, 1064) 'ref.tmp94' (line 107)
    [1088, 1104) 'current' (line 112)
    [1120, 1136) 'agg.tmp125'
    [1152, 1168) 'agg.tmp135'
    [1184, 1192) 'p' (line 123)
    [1216, 1256) 'agg.tmp176'
    [1296, 1336) 'agg.tmp179'
    [1376, 1416) '__begin3' (line 136)
    [1456, 1496) '__end3' (line 136)
    [1536, 1544) 'pos197' (line 136)
    [1568, 1600) 'ref.tmp205' (line 136)
    [1632, 1696) 'steps' (line 147)
    [1728, 1736) 'successor' (line 152)
    [1760, 1768) 'ref.tmp625' (line 184)
    [1792, 1800) 'ref.tmp646' (line 187)
    [1824, 1840) 'ref.tmp647' (line 187)
    [1856, 1864) 'ref.tmp675' (line 191)
    [1888, 1904) 'ref.tmp690' (line 193)
    [1920, 1936) 'ref.tmp706' (line 197)
    [1952, 1960) 'ref.tmp725' (line 198)
    [1984, 2024) '__begin1' (line 206)
    [2064, 2104) '__end1' (line 206)
    [2144, 2152) 'ref.tmp784' (line 208)
    [2176, 2184) 'ref.tmp838' (line 213)
    [2208, 2216) 'ref.tmp841' (line 213)
    [2240, 2256) 'ref.tmp842' (line 213)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/flo/factorio-bot/bot+0x86fea4) 
Shadow bytes around the buggy address:
  0x1000369c2710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000369c2720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000369c2730: 00 00 00 00 00 00 00 00 f2 f2 f2 f2 f2 f2 f2 f2
  0x1000369c2740: f8 f8 f8 f8 f2 f2 f2 f2 f8 f2 00 00 f2 f2 00 00
  0x1000369c2750: 00 00 00 00 00 00 00 00 00 00 00 f2 f2 f2 f2 f2
=>0x1000369c2760: f8 f2 f2 f2 00 00 00 00 00 f2 f2[f2]f2 f2 f8 f2
  0x1000369c2770: 00 00 00 00 00 00 00 f2 f2 f2 f2 f2 f8 f8 f2 f2
  0x1000369c2780: f8 f2 f2 f2 00 00 f2 f2 00 00 f2 f2 00 00 f2 f2
  0x1000369c2790: f8 f2 f2 f2 00 00 00 00 00 f2 f2 f2 f2 f2 00 00
  0x1000369c27a0: 00 00 00 f2 f2 f2 f2 f2 f8 f8 f8 f8 f8 f2 f2 f2
  0x1000369c27b0: f2 f2 f8 f8 f8 f8 f8 f2 f2 f2 f2 f2 f8 f2 f2 f2
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==2832==ABORTING

Miners should only be placed on "pure" fields, and not mix different resources.

When taking stuff from furnaces, it should be the product inventory!

Possible fixes:

1. Make the bot aware of the different inventories, so the right one can be selected
2. hard-code the inventory based on the entity type / item type (e.g.: wood from chest -> chest inventory. wood from furnace: fuel inventory. iron-plate from furnace: ???)
3. Ignore the problem and make the decision in the lua mod.

Best would be 1.