[2.0.44] sort_and_merge breaks ammo inventories

[SunBlade]

Issue

when using LuaInventory::sort_and_merge on an ammo inventory, ammunition gets possibly sorted away from their intended weapons. the gui does not allow that.
here an example with the player character and a tank.
this:

ammo_normal.png (41.72 KiB) Viewed 278 times

becomes this:

ammo_broken.png (40.28 KiB) Viewed 278 times

How to reproduce

1. load the supplied example save with a prepared tank and some weapons and ammo.
2. execute the following command.

   Code: Select all

   /c local function sort_inventories(ntt)
   	local get = ntt.get_inventory
   	for n = 1,ntt.get_max_inventory_index(),1 do
   		local i = get(n)
   		if i and #i>0 then
   			i.sort_and_merge()
   		end
   	end
   end
   sort_inventories(game.player.character)
   sort_inventories(game.player.vehicle)

3. almost all weapons are unusable now since their ammo is in the wrong slot.

Bonus

After fixing the issue, this command intentionally breaks player ammo on first run by sorting weapons after ammo is sorted.
A second run should then fix ammo again.

Code: Select all

/c local function sort_inventories(ntt)
	local get = ntt.get_inventory
	for n = ntt.get_max_inventory_index(),1,-1 do
		local i = get(n)
		if i and #i>0 then
			i.sort_and_merge()
		end
	end
end
sort_inventories(game.player.character)
sort_inventories(game.player.vehicle)

i think this counts as intended behaviour, but while you are at it:
if you can find a simple solution to link ammo slots to their weapons, i think nobody would complain.

[boskid]

Wont fix.

Most player operations with inventory stacks go through owner entity that adds special rules to what can go where. sort_and_merge goes directly to inventory and asks it to sort regardless of entity owner rules. Due to all optimization that went into inventory sort, if it would be now required to query each item if it can go to every stack that would degrade into O(N^2) affecting character main inventory sorting just because ammo could have extra rules. Since i do not want this, the only reasonable fix would be to ban sort requests on those inventories. This would also prevent sorting modules inventory, crafter ingredients inventory, crafters products inventory and many more.

If you do not want those inventories scrambled, simply do not call sort on them, this type of sorting with special rules per slot was never implemented except of inventory with filters where rules follow a common structure so they can be partially resolved into groups making sort still be fast.

[SunBlade]

Due to all optimization that went into inventory sort, if it would be now required to query each item if it can go to every stack that would degrade into O(N^2) affecting character main inventory sorting just because ammo could have extra rules.

using merge sort or timsort your raw sorting complexity is at worst O(NlogN), but the resulting pattern can be invalid according to the entity owner rules. to get a valid pattern you would need to resort to a bruteforce bubble sort to query entity owner rules each step of the way. yea, that really sounds like a terrible idea.

... player operations with inventory stacks go through owner entity that adds special rules ...
... except of inventory with filters ...

i assumed those entity owner rules were additional filter rules imposed by the inventory. does that also mean scripts can insert/transfer stuff into inventories they where never designed for, like iron plates into fuel slots or spoilage into blueprint books? or is sort_and_merge the only one with too much item stack interaction that it needs to circumvent entity owner rules?

... sort_and_merge goes directly to inventory and asks it to sort regardless of entity owner rules. ...
... simply do not call sort on them ...

ok, got it, build a blocklist and check every inventory against it. are ammo inventories the only ones with heavy entity owner rules per slot, or are there more i should look out for?


if i detect such a special needs inventory, would this be a valid sorting strategy:

1. put all items in a temporary sorting inventory.
2. call sort_and_merge on that inventory.
3. fill up the inventory again using find_empty_stack and transfer_stack in an orderly fashion.

if sort_and_merge is not the only function to ignore entity owner rules, what is with these functions:

• LuaInventory::can_insert
• LuaInventory::count_empty_stacks
• LuaInventory::find_empty_stack
• LuaInventory::get_insertable_count
• LuaInventory::insert
• LuaItemStack::import_stack
• LuaItemStack::set_stack
• LuaItemStack::swap_stack (both ways?)
• LuaItemStack::transfer_stack

[SunBlade]

i did some testing. as boskid mentioned above, some entities already have sorting turned off. to be more specific: if a prototype.type can potentially have a main or trash inventory, only then is sorting allowed. that means: crafting machines, beacons, reactors and the sorts can not be sorted with sort_and_merge at all, while tanks, chests, silos and the sorts allow indiscriminate sorting with sort_and_merge on all their inventories.
edit: my mod somehow interfered with that, in vanilla factorio ALL inventories are sortable. so you can mess up weapons, ammunition, ingredients, outputs and many more.

functions which work on single stacks can be divided into two categories: query functions and action functions. while query functions ignore entity owner rules, action functions obey entity owner rules. for example: while find_empty_stack thinks it a good idea to insert scrap into the module slots of a crafter, a following transfer_stack will then complain and not do it. the other functions follow the same scheme. any function trying to move or create an item stack checks entity owner rules. any function "just" looking at the inventory ignores entity owner rules.