Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich item copy for inventory drops #1346

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Rich item copy for inventory drops #1346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2b8d570
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
c75d668
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
b4b3684
Added item filtering for offline corpse dumps
Soggs Feb 10, 2023
afdb8f1
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
1836ef6
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
ae76d9f
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
91dc1f9
Update dump_offline_inventories.lua
Soggs Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ global.config = {
dump_offline_inventories = {
enabled = false,
offline_timout_mins = 15, -- time after which a player logs off that their inventory is provided to the team
ignored_items = {} -- list of prototype names that remain in the player inventory
},
-- enables players to create and prioritize tasks
tasklist = {
Expand Down
48 changes: 21 additions & 27 deletions features/dump_offline_inventories.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,30 @@ local Config = require 'config'
local set_timeout_in_ticks = Task.set_timeout_in_ticks
local config = Config.dump_offline_inventories

local ignored_items_set = {}
for _ , k in pairs(config.ignored_items) do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for _ , k in pairs(config.ignored_items) do
for _ , k in pairs(config.ignored_items or {}) do

There are a lot maps that directory set dump_offline_inventories with a new table that are missing this new field, search for dump_offline_inventories =. Another option would be to go to all those sites and add an empty table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add that.

ignored_items_set[k] = true
end

local offline_player_queue = {}

Global.register({offline_player_queue = offline_player_queue}, function(tbl)
offline_player_queue = tbl.offline_player_queue
config = Config.dump_offline_inventories
end)

local function move_items(source, target)
if not source.is_empty() then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not source.is_empty() then
if source.valid and not source.is_empty() then

I think with the way you changed the checks it's technically possible to get here with only one of the inventories being valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we never let go of control between obtaining the reference and using it, it should never become invalid. However we do need a nil check at that point. Which gives me an idea to rework the whole control flow of how we handle things. So I will come back to it.

for i = 1, #source do
local stack = source[i]
if stack.valid_for_read and not stack.is_selection_tool() and not stack.is_blueprint_book() and not ignored_items_set[stack.name] then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if stack.valid_for_read and not stack.is_selection_tool() and not stack.is_blueprint_book() and not ignored_items_set[stack.name] then
if stack.valid and stack.valid_for_read and not stack.is_selection_tool and not stack.is_blueprint_book and not ignored_items_set[stack.name] then

According to the API these are boolean values not functions.
https://lua-api.factorio.com/latest/LuaItemStack.html#LuaItemStack.is_selection_tool
https://lua-api.factorio.com/latest/LuaItemStack.html#LuaItemStack.is_blueprint_book

Also I think bizarrely you have to check valid as well as valid for read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got carried over by the is_empty()

As for stack being invalid. That would imply that the inventory we pull from contains invalid references which should never be an constitute a major bug in Factorio.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be an edge case if the stack gets cleared on the same tick by another event. The stack might still be there but not valid? I'm not sure, but the Factorio devs always recommend adding the valid check and in this case there is no performance concern and it doesn't make the code more complicated. So, I'd be happier with the check, same above for the inventory check.

target.insert(stack)
stack.clear()
end
end
end
end

local function spawn_player_corpse(player, banned, timeout_minutes)
local player_index = player.index
offline_player_queue[player_index] = nil
Expand All @@ -29,25 +46,13 @@ local function spawn_player_corpse(player, banned, timeout_minutes)
local inv_main = player.get_inventory(defines.inventory.character_main)
local inv_trash = player.get_inventory(defines.inventory.character_trash)

local inv_main_contents
if inv_main and inv_main.valid then
inv_main_contents = inv_main.get_contents()
end

local inv_trash_contents
if inv_trash and inv_trash.valid then
inv_trash_contents = inv_trash.get_contents()
end

local inv_corpse_size = 0
if inv_main_contents then
if inv_main and inv_main.valid and not inv_main.is_empty() then
inv_corpse_size = inv_corpse_size + (#inv_main - inv_main.count_empty_stacks())
end

if inv_trash_contents then
if inv_trash and inv_trash.valid and not inv_trash.is_empty() then
inv_corpse_size = inv_corpse_size + (#inv_trash - inv_trash.count_empty_stacks())
end

if inv_corpse_size <= 0 then
return
end
Expand All @@ -63,19 +68,8 @@ local function spawn_player_corpse(player, banned, timeout_minutes)

local inv_corpse = corpse.get_inventory(defines.inventory.character_corpse)

for item_name, count in pairs(inv_main_contents or {}) do
inv_corpse.insert({name = item_name, count = count})
end
for item_name, count in pairs(inv_trash_contents or {}) do
inv_corpse.insert({name = item_name, count = count})
end

if inv_main_contents then
inv_main.clear()
end
if inv_trash_contents then
inv_trash.clear()
end
move_items(inv_main, inv_corpse)
move_items(inv_trash, inv_corpse)

local text = player.name .. "'s inventory (offline)"
local tag = player.force.add_chart_tag(player.surface, {
Expand Down