ID:2519774
 
BYOND Version:512
Operating System:Windows 10 Home 64-bit
Web Browser:Firefox 69.0
Applies to:Dream Daemon
Status: Open

Issue hasn't been assigned a status value.
Descriptive Problem Summary:
Inserting an non-null icon into a blank icon (created from a blank .dmi with no states) throws "Bad icon operation" runtimes.

Runtime in detail:
[13:00:00] Runtime in ,: bad icon operation
proc name: Insert (/icon/proc/Insert)
usr: Irrationalist (/mob/new_player) ([0x3000001]) (NULL) (0,0,0) (NULL) (irrationalist)
usr.loc: *null*
src: /icon (/icon)
call stack:
/icon (/icon): Insert(/icon (/icon), null, 8, null, null, null)
/datum/rick/mob_icon_compiler (/datum/rick/mob_icon_compiler): generate(Angel Gibson (mannequin) (/mob/living/carbon/human/dummy/mannequin))
Angel Gibson (mannequin) (/mob/living/carbon/human/dummy/mannequin): update body(0)
Angel Gibson (mannequin) (/mob/living/carbon/human/dummy/mannequin): force update limbs()
/datum/preferences (/datum/preferences): copy to(Angel Gibson (mannequin) (/mob/living/carbon/human/dummy/mannequin), 1)
/datum/preferences (/datum/preferences): dress preview mob(Angel Gibson (mannequin) (/mob/living/carbon/human/dummy/mannequin))
/datum/preferences (/datum/preferences): update preview icon()
Body (/datum/category_item/player_setup_item/physical/body): content(Irrationalist (/mob/new_player))
Physical (/datum/category_group/player_setup_category/physical_preferences): content(Irrationalist (/mob/new_player))
/datum/category_collection/pla... (/datum/category_collection/player_setup_collection): content(Irrationalist (/mob/new_player))
/datum/preferences (/datum/preferences): ShowChoices(Irrationalist (/mob/new_player))
Irrationalist (/mob/new_player): Topic("src=\[0x3000001];show_preferen...", /list (/list))
Irrationalist (/client): Topic("src=\[0x3000001];show_preferen...", /list (/list), Irrationalist (/mob/new_player))
Irrationalist (/client): Topic("src=\[0x3000001];show_preferen...", /list (/list), Irrationalist (/mob/new_player))

Numbered Steps to Reproduce Problem:
1. Create a new icon from a blank .dmi
2. Try to insert an other icon (be it from an other dmi or a procedurally generated one using several icons clobbered together with Blend, etc.), doesn't matter if it is blank or not.
Code Snippet (if applicable) to Reproduce Problem:
// Not included are two strings that turn the blend styles and 
// directions into readable strings for debugging output

/datum/rick/mob_icon_compiler/generate(var/mob/living/carbon/human/H)
var/icon/output_icon = new/icon('modules/ric/blanks.dmi')

for(var/direction in list(NORTH,SOUTH,WEST,EAST))
// A procedurally generated (and seemingly valid) icon
var/icon/dir_base_icon = compile_direction(direction, H)

// Trying to insert non-blanks from files also cause runtimes
// dir_base_icon = new/icon('icons/mob/human.dmi',"body_m_s")

src.log_event("Inserting [direction_to_string(direction)] into output")
output_icon.Insert(new/icon(dir_base_icon, dir = direction), dir = direction)

src.log_event("Done!")

return output_icon

/datum/rick/mob_icon_compiler/proc/compile_direction(var/direction, var/mob/living/carbon/human/H)
var/obj/item/organ/external/root = H.get_organ(BP_CHEST)
var/icon/base_icon = new/icon(root.get_icon(), dir = direction)

src.log_event("Collecting body part info.")
for(var/obj/item/organ/external/part in (H.organs-root))
var/datum/arg_object/mob_icon_part/O = part.visit_mob_icon_part(direction, list())
if (O && O.body_icon)
base_icon.Blend(O.body_icon, O.blend_style)

src.log_event("Blended [part.organ_tag] into [direction_to_string(direction)] component with [blend_to_string(O.blend_style)]")
return base_icon


Expected Results:
No runtime.
Actual Results:
Runtime with a very insightful description helpful to solving the problem painlessly with

Does the problem occur:
Every time? Or how often?
Every time when this operation is run
In other games?
N/A
In other user accounts?
N/A
On other computers?
N/A

When does the problem NOT occur?
I've yet to figure out.

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? (Visit http://www.byond.com/download/build to download old versions for testing.)
Unknown

Workarounds:
By not using Insert()

Besides, on a more serious note, only this seems to work without runtimes.
var/obj/item/organ/external/chest = get_organ(BP_CHEST)
base_icon = chest.get_icon()

for(var/obj/item/organ/external/part in (organs-chest))
var/icon/temp = part.get_icon()
//That part makes left and right legs drawn topmost and lowermost when human looks WEST or EAST
//And no change in rendering for other parts (they icon_position is 0, so goes to 'else' part)
if(part.icon_position & (LEFT | RIGHT))
var/icon/temp2 = new('icons/mob/human.dmi',"blank")
temp2.Insert(new/icon(temp,dir=NORTH),dir=NORTH)
temp2.Insert(new/icon(temp,dir=SOUTH),dir=SOUTH)
if(!(part.icon_position & LEFT))
temp2.Insert(new/icon(temp,dir=EAST),dir=EAST)
if(!(part.icon_position & RIGHT))
temp2.Insert(new/icon(temp,dir=WEST),dir=WEST)
base_icon.Blend(temp2, ICON_OVERLAY)
if(part.icon_position & LEFT)
temp2.Insert(new/icon(temp,dir=EAST),dir=EAST)
if(part.icon_position & RIGHT)
temp2.Insert(new/icon(temp,dir=WEST),dir=WEST)
base_icon.Blend(temp2, ICON_UNDERLAY)
else if(part.icon_position & UNDER)
base_icon.Blend(temp, ICON_UNDERLAY)
else
base_icon.Blend(temp, ICON_OVERLAY)

Where temp are non-blanks, and temp2 is a blank, the very same circumstances as above.
Can you provide a test project for this?
I've created a project specifically for trying a similar design I had when the Runtimes occurred, along with other test cases to explore what usages work and which do not.

But the problem is that I met no issues when generating these icons in a completely barebones project.

world/New()
world.log << "IconInsertBase()."
IconInsertBase()
IconInsertBase(base_icon_name = 'blanks.dmi', base_iconstate = "blank-d4")
IconInsertBase(base_icon_name = 'parts.dmi')
IconInsertBase(base_icon_name = null)
IconInsertBase(parts_to_blend_in = 4)

world.log << "IconInsertNone()."
IconInsertNone()

world.log << "IconInsertBlended()."
IconInsertBlended()

world.log << "Done icon operations."

proc/IconInsertNone()
var/icon/base = new()

// Blend into for one single direction
var/icon/target = new()

// Insert the result into the base
base.Insert(target)

return base

proc/IconInsertBase(var/base_icon_name = 'empty.dmi', \
var/base_iconstate = null,\
var/parts_target_icon_name = 'blanks.dmi',\
var/parts_icon_name = 'parts.dmi',\
var/parts_to_blend_in = 1)
var/icon/base = new(base_icon_name, base_iconstate)

// Blend into for one single direction
var/icon/target = new(parts_target_icon_name, dir = NORTH)

for (var/idx = 1 to parts_to_blend_in)
target.Blend(new/icon(parts_icon_name,"[(idx % 4) + 1]", dir = NORTH), ICON_OVERLAY)

// Insert the result into the base
base.Insert(target)

return base

proc/IconInsertBlended(var/base_icon_name = 'empty.dmi', \
var/base_iconstate = null,\
var/parts_target_icon_name = 'blanks.dmi')
var/icon/base = new(base_icon_name, base_iconstate)

// Blend into for one single direction
var/icon/target = new(parts_target_icon_name)

for(var/direction in list(NORTH, SOUTH, EAST, WEST))
target.Insert(new/icon(CreateBlendedIcon(), dir = direction), dir = direction)

// Insert the result into the base
base.Insert(target)

return base

proc/CreateBlendedIcon()
var/icon/result = new ()

result.Blend(new/icon('parts.dmi', "1"), ICON_OVERLAY)
result.Blend(new/icon('parts.dmi', "2"), ICON_UNDERLAY)
result.Blend(new/icon('parts.dmi', "3"), ICON_OVERLAY)
result.Blend(new/icon('parts.dmi', "4"), ICON_UNDERLAY)

return result


Regardless, I've uploaded the project I made this morning to test and see if the problem could be replicated. https://github.com/martinlyra/IconInsertTest

Though, I am afraid that this seems to be a bug caused by either me writing the icon generation in the wrong way or a technical debt commonly seen in Space Station 13 projects. I don't understand.

Besides, if it helps, the very specific version I am using is 512.1488
To be honest I don't think I can progress on this until there's a test case that reliably reproduces the problem.

Since you're having issues in SS13 itself, I would suggest loading the icon ops around the problem area down with debug statements that spit data out to a log. See what the value is of the icon you're inserting and what you're inserting into (using \ref, and maybe also \ref[icon.icon] if the icon is a valid /icon datum), and that might lead you back to where something goes wrong.
Sounds like an idea. I'll run some more tests and try to narrow down the issue.

I did some tests earlier the same morning before leaving for an exam.
There are some conclusions so far

Runtimes are thrown when:
- The icon being inserted is returned by the function "compile_direction", whether the icon is blended, directly from a file, or completely blank (new/icon(null) and new/icon()) does not matter.
- The icon being inserted has been created in the same scope but has been blended at least once.

Runtimes are not thrown when:
- The icon has been created in the same scope as the Insert function. Doesn't matter if blank, from file, or fetched from an other function that returns an icon. Functions from non-datum objects seem to work just fine.

Though, I've nothing to back up these observations I've seen so far.

Which begs me to question for more ways to debug this further. What are the good ways to see the value of an icon?
fcopy it into an output dmi?
Good news. I've finally figured a way to around it. It appears so that in my problem (working against a rather large SS13 codebase) you've a narrow time window before the icon goes "stale" and runtimes when you try to modify it with Insert.

In my original problem, I created the icon before doing a, relatively speaking, time-intensive process of collecting icon data and their /icon datums. It appeared so that it took long enough to finish for one direction. By when it reaches to the output icon for insertion, it has gone "stale" and refuses further operations.

I tried moving the creation of the output icon inside the same loop where it collects the info.

Such that...
var/icon/output_icon = icon(...)
//...
for(var/direction in list(NORTH,SOUTH,WEST,EAST))
var/icon/dir_base_icon = compile_direction(direction, H)

output_icon.Insert(dir_base_icon, dir= direction)

... becomes.
var/icon/output_icon
//...
for(var/direction in list(NORTH,SOUTH,WEST,EAST))
var/icon/dir_base_icon = compile_direction(direction, H)

if (!output_icon)
output_icon = icon(...)
output_icon.Insert(dir_base_icon, dir= direction)


Inserting an icon inside it worked. But only once - only in the loop it was created in, since the next loops took too long time and by when DM reached to the next Insert, it had gone stale.

The final solution became doing the insertions as one last thing. Aka do the intensive work first, then Insert the icons together right after the creation of the output icon that I wished to return. The compile_direction function remains unchanged.
/datum/rick/mob_icon_compiler/generate(var/mob/living/carbon/human/H)
var/icon/output_icon

src.log_event("Collecting body part info.")

var/list/directions = list(NORTH,SOUTH,WEST,EAST)
var/list/components = list()

for(var/direction in directions)
try
var/icon/dir_base_icon = compile_direction(direction, H)
src.log_event("Debug: Saving result icon to RICK_output_[direction].dmi ... Result: [fcopy(dir_base_icon, "RICK_output_[direction].dmi")]")
components["[direction]"] = dir_base_icon
catch (var/exception/e)
src.log_event("[direction_to_string(direction)] has error'd with [e] on [e.file]:[e.line]. Skipping direction.")

for(var/direction in components)
try
if (!components[direction])
continue
if (!output_icon)
output_icon = icon(icon= 'modules/ric/blanks_32.dmi', icon_state= "")
output_icon.Insert(components[direction], dir= text2num(direction))
catch (var/exception/e)
src.log_event("ERR: Couldn't Insert() compononent of [direction_to_string(text2num(direction))] to output. Cause: [e]")

if (output_icon)
src.log_event("Debug: Saving output icon to RICK_output_post.dmi ... Result: [fcopy(output_icon, "RICK_output_post.dmi")]")
src.log_event("Debug: Post-generate '[output_icon]' (\ref[output_icon]), '[output_icon.icon]' (\ref[output_icon.icon])")
else
src.log_event("ERR: Compile of icon for [H] (\ref[H]) has failed. Returning null.")

src.log_event("Done!")

return output_icon

To abstractify it:
/datum/rick/mob_icon_compiler/generate(var/mob/living/carbon/human/H)
var/icon/output_icon

var/list/directions = list(NORTH,SOUTH,WEST,EAST)
var/list/components = list()

// Collect info and icons
for(var/direction in directions)
var/icon/dir_base_icon = compile_direction(direction, H)
// ...
components["[direction]"] = dir_base_icon

// Connect them all together at once
for(var/direction in components)
if (!components[direction])
continue
if (!output_icon) // Lazy access
output_icon = icon(icon= 'modules/ric/blanks_32.dmi', icon_state= "")
output_icon.Insert(components[direction], dir= text2num(direction))
// ...
return output_icon


If this "time window" was intended, then the runtime description is sorely unhelpful if not misguiding. There is also no documentation on it.

If not, then perhaps: Would the actual issue lie somewhere in that icons cannot perform Insert operations, if the free memory/cache space ahead of it got occupied by other functions? Like in this case, by other operations by the relatively intensive function compile_direction and probably other tasks run by SS13 in the background?
Interesting result. What does compile_direction() do exactly? Can you show me that code?

One possibility here is that you might be overloading the runtime dynamic icon cache: the server is only intended to keep so many /icon datums open and editable at once. In theory this shouldn't lead to anything so insidious as a crash, but I suppose that's a possibility given the results you're seeing.

I'll have to review the code to see if I can find an explanation for why this kind of thing might occur, but first it'd help to look at compile_direction() to see.
Functionally, they remain unchanged.
This is the old function that was originally in the OP at start of thread.
/datum/rick/mob_icon_compiler/proc/compile_direction(var/direction, var/mob/living/carbon/human/H)
var/obj/item/organ/external/root = H.get_organ(BP_CHEST)
var/icon/base_icon = new/icon(root.get_icon(), dir = direction)

src.log_event("Collecting body part info.")
for(var/obj/item/organ/external/part in (H.organs-root))
var/datum/arg_object/mob_icon_part/O = part.visit_mob_icon_part(direction, list())
if (O && O.body_icon)
base_icon.Blend(O.body_icon, O.blend_style)

src.log_event("Blended [part.organ_tag] into [direction_to_string(direction)] component with [blend_to_string(O.blend_style)]")
return base_icon


This is the function in its current state
/datum/rick/mob_icon_compiler/proc/compile_direction(var/direction, var/mob/living/carbon/human/H)
var/icon/base_icon = icon(icon= 'modules/ric/blanks_32.dmi', icon_state= "d1")
//src.log_event("Debug: Pre-compile '[base_icon]' (\ref[base_icon]), '[base_icon.icon]' (\ref[base_icon.icon])")

for(var/obj/item/organ/external/part in (H.organs))
try
var/datum/arg_object/mob_icon_part/O = part.visit_mob_icon_part(direction, list())
if (O && O.body_icon)
base_icon.Blend(new/icon(O.body_icon), O.blend_style)

//src.log_event("Blended [part.organ_tag] into [direction_to_string(direction)] component with [blend_to_string(O.blend_style)]")
catch (var/exception/e)
src.log_event("ERR: Dir:[direction_to_string(direction)] Tag:[part.organ_tag] \"[e]\" on [e.file]:[e.line]. Skipping part.")

//src.log_event("Debug: Post-compile '[base_icon]' (\ref[base_icon]), '[base_icon.icon]' (\ref[base_icon.icon])")
return base_icon

Indeed, the base icon it blends against is now a blank, but I want to assert that it had no effect on the problem. It was a change that I did as a convenience and attempt to try around the problem.

The odd part is that all it does is fetch one or more icons (currently only one, the function is overridable, however) from parts of the human mob's body and blend it into the icon for the direction it is generating.

In turn, the function that it gets the icon from is this snippet of code (it returns the icon inside an object)

/datum/arg_object/mob_icon_part
var/icon/body_icon
var/icon/cloth_icon
var/blend_style = ICON_OVERLAY

/obj/item/organ/external/proc/visit_mob_icon_part(var/direction, var/list/args)
var/datum/arg_object/mob_icon_part/O = new /datum/arg_object/mob_icon_part()

O.body_icon = new/icon(get_icon(), dir= direction)
O.cloth_icon = new/icon(get_icon(), dir= direction)
O.blend_style = get_preferrable_blend(direction)

return O

/obj/item/organ/external/proc/get_preferrable_blend(var/direction = src.dir)
if (icon_position & UNDER)
return ICON_UNDERLAY

if (icon_position & RIGHT || icon_position & LEFT)
switch(direction)
if (EAST)
if (icon_position & RIGHT)
return ICON_OVERLAY
else
return ICON_UNDERLAY
if (WEST)
if (icon_position & LEFT)
return ICON_OVERLAY
else
return ICON_UNDERLAY
else

return ICON_OVERLAY


Where the icon it returns is generated by this function.
https://github.com/UristMcStation/UristMcStation/blob/ 9a21f136f4c23c38abd7c4420e2f1516af3f2cad/code/modules/ organs/external/_external_icons.dm#L120

How much deeper you are willing to go is up to you. But this is the codebase I am working against, and currently, this project (all code snippets provided, except the link) is in private as I feel like I am not yet done with it.